-
Notifications
You must be signed in to change notification settings - Fork 21
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
only_warn should produce warnings not errors #78
Comments
What do you mean by "Now"? Did anything change recently? Since when? How do you set |
now == last time i've tested it - few days ago at latest version I believe It works well as action
The only minor issue - one i'm talking about - is only_warn makes codespell to not fail action run by using return code 0. |
I see. So it's not a new or recent issue. The word "New" simply means you're running the latest version. |
I have no clue how to fix this myself. If you know, please do not hesitate to provide a pull request. |
But then, it looks like
|
Yeah i just suggest to replace all error annotations as warnings. Sadly I don't fully understand how this action works so I can't suggest proper fix. Looks like
|
But that's not the documented behaviour as far as I an tell. That would be a breaking change involving a major version bump, wouldn't it? On the other hand, that's what other actions do. See for example https://github.com/actions/dependency-review-action:
Finally, what exact difference does that make in practice? How do warnings and errors differ in the output of CI jobs? |
TBH I see that matcher json for first time and I didn't even see docs of how to use it. Usually i just write something like |
So I based this action on this one and just took the names from there: FWIW the matcher stuff is documented here: The main benefit is you don't need to make any changes to the tool generally, you just write some Regex to match its output (which also makes things like https://github.com/codespell-project/sort-problem-matcher possible where they probably wouldn't accept a PR to modify sort for GitHub usage). Unfortunately if the docs are correct you can't just overwrite the number in that config as they Regex group matches, you'd have to either modify Codespell itself or pre-process its output. Personally I'd say there's an argument for both use cases, you might want to flag the correct severity of found issues, whilst not failing the whole build (as it currently does) but maybe in retrospect that should have been a differently named option. Then there's what you're asking for where all logged alerts are at the warning level. But do you also want the job to be forced success too? |
Correction, apparently their docs are rubbish and you can sort of override severity, but you'd need a different matcher: |
Now all messages show as ::error regardless of only_warn value.
The text was updated successfully, but these errors were encountered: