Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add output variable so workflows can detect linter problems #93

Closed
wants to merge 2 commits into from

Conversation

markphip
Copy link

@markphip markphip commented Oct 9, 2020

This adds an output variable named problems_detected that will be set to true if one or more of the linters find problems OR if an autofix event creates and pushes a code change back to the pull request. The README.md was also updated to document this variable and provide an example of its usage. This would close issue #60

This is set to true if a linter has errors or if any code
is autofixed and committed. This allows the workflow
to do something different based on the linter outcome.
Also provides an example of how to use it.
@markphip markphip changed the title Add output variable to workflows can detect linter problems Add output variable so workflows can detect linter problems Oct 9, 2020
@ocean90
Copy link
Member

ocean90 commented Oct 11, 2020

Thank you for the detailed pull request! I like the idea of using output variables for this but I'm wondering if we should provide one for each linter since it's possible to run multiple linters in one step. Just like the input variable we could prefix it with the later name, for example black_problems_detected or eslint_problems_detected. Thoughts?

Once I get the tests working again and #91 is merged, we can also use @actions/core.

@markphip
Copy link
Author

I have started using this and have already thought that at minimum there should be separate flags for autofix and linter problems. I would not be at all opposed to one flag per linter. I will say that I have already exceeded my capabilities here as once I get past copy and pasting some Javascript I am out of my element. So hopefully someone else can pick this up and turn it into a worthy feature to include. The basic concept is working well for m though.

@ocean90 ocean90 added the enhancement New feature or request label Oct 18, 2020
@ocean90
Copy link
Member

ocean90 commented Nov 30, 2020

@markphip Do you think this is still helpful now that we have continue_on_error #124?

@markphip
Copy link
Author

I have not tried it yet, but this would have met my needs. I like that it is simpler and addresses what most people probably want. My solution was mainly because I did not think you wanted it to work this way (or I did not think of it). There is some potential value in the approach I took to handle more complex scenarios ... but I will likely abandon what I did and switch to this at some point because I really just wanted to end the job when there were errors.

@ocean90
Copy link
Member

ocean90 commented Nov 30, 2020

Yeah, it's actually a decision against the initial design of the action. But since GitHub currently doesn't provide an easy way to take action on the result of check runs I decided to go with continue_on_error for an improved developer experience.

@markphip markphip closed this Jan 2, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants