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

lint: bring back sentry.lint.sentry_check:SentryCheck #9053

Merged
merged 4 commits into from
Jul 18, 2018

Conversation

joshuarli
Copy link
Member

Upgrading to flake8 3.5.0 caused pycodestyle's register_check to silently ignore our custom checker, so we're missing out on some linting. Fortunately, flake8 3.5.0 lets us use local plugins via setup.cfg as documented here.

@joshuarli joshuarli requested a review from mattrobenolt July 16, 2018 18:39
@mattrobenolt
Copy link
Contributor

So after being extremely mad at computers, this really is the only sensible way to do it now. There's not a way to programmatically register extensions/plugins without digging really deep into things that flake8 does not want you to dig deep into and would be too much of a hack.

but this also means that this snippet of config needs to exist in all repos that try to use our checks since it won't inherit from the sentry repo in my testing.

And lastly, now that it's actually running our checks, it points out linter violations that you introduced in a commit while linters weren't running. So fix that up. :)

@mattrobenolt
Copy link
Contributor

Scanning all the repos I have, the only one that seems to actually do this ouside of this one are:

getsentry/getsentry and getsentry/sentry-plugins

@joshuarli
Copy link
Member Author

@mattrobenolt so flake8 doesn't yet support file-level error ignores. It's currently possible with an additional plugin, but we probably don't want more stuff. I'd have to inline # noqa: B314 for every print statement (there are 15) in our lint engine. Or can just disable linting entirely on that one file with # flake8: noqa. What do you say?

@joshuarli joshuarli force-pushed the lint/fix-sentry-check branch from 6aa983a to 41e41a7 Compare July 18, 2018 18:04
@mattrobenolt
Copy link
Contributor

Right. Just don’t use print. You can use like,

def echo(s, err=False):
  fp = sys.stderr if err else sys.stdout
  fp.write(s)
  fp.flush()

@joshuarli
Copy link
Member Author

Note: we discussed this briefly outside of the issue and we're just going to noqa the print lines.

@joshuarli
Copy link
Member Author

So the currently pinned pycodestyle 2.3.0 in #9000 is kind of buggy. For example:

(sentry) josh@akashi.local sentry lint/fix-sentry-check $ bin/lint --python src/sentry/deletions/__init__.py
/Users/josh/sentry/sentry/src/sentry/deletions/__init__.py:29:1: E302 expected 2 blank lines, found 1
/Users/josh/sentry/sentry/src/sentry/deletions/__init__.py:37:5: E306 expected 1 blank line before a nested definition, found 0
/Users/josh/sentry/sentry/src/sentry/deletions/__init__.py:38:5: E306 expected 1 blank line before a nested definition, found 0
/Users/josh/sentry/sentry/src/sentry/deletions/__init__.py:39:5: E306 expected 1 blank line before a nested definition, found 0
/Users/josh/sentry/sentry/src/sentry/deletions/__init__.py:40:5: E306 expected 1 blank line before a nested definition, found 0
/Users/josh/sentry/sentry/src/sentry/deletions/__init__.py:41:5: E306 expected 1 blank line before a nested definition, found 0
... like 50 more lines of this

Bumping to 2.3.1 fixes this, but the latest 2.4.0 does not work with flake8 3.5.0.

@joshuarli joshuarli merged commit 2a3c9d4 into master Jul 18, 2018
@joshuarli joshuarli deleted the lint/fix-sentry-check branch July 18, 2018 19:21
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants