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

Analyzer Warnings are Not considered Errors #128

Closed
grantjk opened this issue Sep 25, 2015 · 5 comments
Closed

Analyzer Warnings are Not considered Errors #128

grantjk opened this issue Sep 25, 2015 · 5 comments

Comments

@grantjk
Copy link

grantjk commented Sep 25, 2015

When the static analyzer fails on the server, the report sent to Github should be a failure status. Currently, the status is reported as green, with no mention of the analyzer errors

Example PR with failing analyzer

screenshot 2015-09-24 22 02 20

PR Status Message

screenshot 2015-09-24 22 02 47

@grantjk
Copy link
Author

grantjk commented Sep 25, 2015

I realized later tonight that analyzer warnings are reported if there are no other warnings. IMO, I still think that if the build comes back with warnings and/or analyzer warnings, this should be posted as a "failed" state to Github so the red x shows up rather than the green checkmark

@czechboy0
Copy link
Member

Would you consider a warning a reason to also fail? Or would you treat analyzer warnings and warnings differently?

You can see here that warnings and analyzer warnings as still treated as success (I've seen situations when migrating Xcode's, e.g., when a couple of warnings will be in the project for a couple of days and there's not much you can do about it). I just added analyzer warnings there as well, but I don't have any strong feelings about it - I could move it to the failure case as well.

So again - is that what you expected? Analyzer warnings being treated as a failure and warnings as success?

@grantjk
Copy link
Author

grantjk commented Sep 25, 2015

Ha! That probably depends who you ask. Like the dot-syntax vs. brackets debate.

Personally, we try to keep all projects with 0 warnings overall, so any warning to me is considered a regression / error. And when I see the green checkmark and Github's big green buttons, it indicates to me that everything is perfect.

Analyzer warnings, however, should definitely be considered failures. Those are things like "You forgot to call super in viewDidLoad which has significant implications.

@czechboy0
Copy link
Member

Fair enough. I don't just want to change it in code, so I'll probably add a way to configure how to treat warnings/analyzer warnings and probably default to the existing behavior, with the option to switch to strict mode.

@czechboy0
Copy link
Member

Moving to #132.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants