-
Notifications
You must be signed in to change notification settings - Fork 109
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
Upgrade versioneer, fix flake8 + other misc fixes #1336
Upgrade versioneer, fix flake8 + other misc fixes #1336
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1336 +/- ##
=======================================
Coverage 84.78% 84.78%
=======================================
Files 82 82
Lines 3372 3372
Branches 999 999
=======================================
Hits 2859 2859
Misses 434 434
Partials 79 79 Continue to review full report at Codecov.
|
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.
for the _version
related problem, we could update our vendored versioneer version: https://github.com/python-versioneer/python-versioneer#updating-versioneer
That should fix it, be backward compatible, and give us an up to date versioneer that is now being maintained by our very own @effigies
cc7a44a
to
4831c7c
Compare
CI fails with:
I don't want to modify the |
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.
you can setup a section on flake8 in our setup.cfg to ignore the versioneer files.
4831c7c
to
852fb7c
Compare
Should be OK for flake8. Not sure how to fix that for pytest:
Is it OK to create a |
Actually not OK for flake8, because as far as I can see, it is run from |
852fb7c
to
d3c2b81
Compare
if you can figure out a minimal solution that involves us having to add a file, that'd still be fine I think. But reusing existing config files would be better :) (if possible) |
Pytest can be configured in setup.cfg. |
I know, but the pytest documentation warns:
Hopefully this is a very simple case 😃 |
979695d
to
b559fcf
Compare
OK, all options have been added to Now I need to tell Code Climate not to bother with |
5becfc2
to
0201bb4
Compare
0201bb4
to
f64fb3d
Compare
See #1341 for shutting up Cold Climate. Hopefully it'll work... |
8a2f758
to
9dd6d07
Compare
Still these codeclimate errors. I just don't know how to exclude the versioneer files. Any clues? The ci/circleci: test_docker failure looks like a more general problem, unrelated to this PR. |
I think code climate only complains on new changes. So after committing the versioneer files initially, it won't be a problem in the next prs. It's not as satisfying, but I don't know how else to fix it. |
I thought #1341 would help:
It should according to the documentation, but doesn't :-( |
d325619
to
881e296
Compare
In 754bf25 I tried whether the exclude patterns work when they are absolute (relative to the .codeclimate.yml file), instead of globs. That didn't work. Now I am wondering whether |
Makes sense. I'm moving it around in #1347. |
This property is overwritten by another property in the same object literal. While I haven't looked much into the code, the fix looks obvious.
Fix LGTM.com error: Suspicious unused loop iteration variable For loop variable 'i' is not used in the loop body.
18d9ef6
to
1be61c0
Compare
Eventually got CodeClimate to work properly by (I think) moving |
Try to mimic examples from: https://docs.codeclimate.com/docs/default-analysis-configuration
1be61c0
to
9b5ebe3
Compare
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.
wonderful, thanks for sticking with this.
* Fix LGTM.com error: Overwritten property This property is overwritten by another property in the same object literal. While I haven't looked much into the code, the fix looks obvious. * Update versioneer: 0.18 → 0.20 Fix LGTM.com error: Suspicious unused loop iteration variable For loop variable 'i' is not used in the loop body. * New attempt to please CodeClimate Try to mimic examples from: https://docs.codeclimate.com/docs/default-analysis-configuration
https://lgtm.com/projects/g/bids-standard/bids-validator/?severity=error
Fix the first of two errors, the fix seems rather obvious.
The other error is in
_version.py
and has been fixed upstream in python-versioneer/python-versioneer#250.