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 & apply pre-commit and lint job #658
Conversation
3220491
to
a1e2a90
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #658 +/- ##
==========================================
- Coverage 81.57% 81.22% -0.35%
==========================================
Files 43 43
Lines 3929 3925 -4
==========================================
- Hits 3205 3188 -17
- Misses 724 737 +13
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
@nipy/team-heudiconv Please review this PR and comment any desired configuration changes. I am using this as the base for several other PRs, so it would be good to get this PR processed quickly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
hang-closing = False | ||
unused-arguments-ignore-stub-functions = True | ||
select = A,B,B902,C,E,E242,F,U100,W | ||
ignore = A003,B005,E203,E262,E266,E501,W503 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick (shouldn't block merge)
IMO it's convenient to comment what these are.
IMHO, I prefer a linter job to a precommit hook. (I haven't used them extensively, so please correct me if I'm missing something.) The reason is that I sometimes want to commit and push a WIP (for example, to commit where I am so I can switch branches to reproduce a bug) and I wouldn't want the linter failure to slow me down. |
That's what |
sort_keys=False) | ||
save_json( | ||
participants_json, | ||
OrderedDict( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note to myself -- we should get rid of them in order of regular dicts -- already ordered AFAIK to the degree necessary - should make this structure more sanely looking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! But wouldn't it cause you conflicts with the work you have done in #656? If not -- feel welcome to merge!
@yarikoptic It will cause conflicts, but that's only a problem for me. |
🚀 PR was released in |
No description provided.