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

Show severity in Github Workflow command message #1055

Merged
merged 4 commits into from Oct 5, 2020

Conversation

JiriSko
Copy link
Contributor

@JiriSko JiriSko commented Sep 14, 2020

Fixes: #1054

Copy link

@rafaelfolco rafaelfolco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This worked for me only when I specify
ansible-lint --parseable-severity roles/test.yml

When I use -p (pep8 format), the severity info is gone.
Is this expected ?

Copy link
Member

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please convert it to draft until you pass all tests.

@ssbarnea ssbarnea added the incomplete Additional work or information is required label Sep 15, 2020
@JiriSko JiriSko marked this pull request as draft September 15, 2020 10:35
@JiriSko JiriSko marked this pull request as ready for review September 16, 2020 11:39
@ssbarnea
Copy link
Member

Code looks ok now but before merging it, do you have a proof it does work? My guess is that github should just ignore the message but it could also prevent it from rendering them.

Can you please made a manual test of producing such an annotation and confirm that it works?

The second question would, what if github does not display severity at all in the UI. How is this feature going to help the user?

I am quite curious about you use-case around priority as I never seen it more than something put inside the rule documentation.

@ssbarnea
Copy link
Member

ssbarnea commented Oct 5, 2020

@JiriSko If you cannot produce an example that proves that github actions would not choke when receiving an additional parameter and that it is possible to view the severity on the annotations produced I will have to close this PR.

Even a screenshot is enough. You should be able to test this with a simple print that produces an annotation that is using this format.

The only reason I am asking this is because I want to prevent a potential break in current code by merging this feature without confirmation that it works.

@JiriSko
Copy link
Contributor Author

JiriSko commented Oct 5, 2020

Sorry it took so long. Here's test with old and new annotation format:
https://github.com/codeacio/ansible-lint/actions/runs/289711321

@ssbarnea
Copy link
Member

ssbarnea commented Oct 5, 2020

@jimi-c No worries. Now we know that github accepts extra data in severity. Still, my impression is that the severity will not be visible to the user. How is user going to make use of it?

@JiriSko
Copy link
Contributor Author

JiriSko commented Oct 5, 2020

It's part of my CI to categorize issues to errors and warnings. Isn't visible directly in Github. I'm using it in previous version of ansible-lint.

@ssbarnea ssbarnea merged commit cff352f into ansible:master Oct 5, 2020
@ssbarnea ssbarnea added enhancement and removed incomplete Additional work or information is required labels Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parseable output including severity and pep8 not working
3 participants