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

Fix E201 and E202 style errors #4440

Merged
merged 3 commits into from Aug 17, 2017

Conversation

Projects
None yet
6 participants
@nsoranzo
Copy link
Member

commented Aug 17, 2017

Remove all whitespaces after ( and before ) and removes E201 and E202 from the ignored linting errors.

As discussed on Gitter/IRC, this brings us closer to the official PEP8 standard and makes our code more uniform.

@mvdbeek

This comment has been minimized.

Copy link
Member

commented Aug 17, 2017

Wonderful, but this is going to get stale super quickly. Happy to merge if you can rebase against the latest changes.

nsoranzo added some commits Aug 16, 2017

Fix all E201 and E202 style errors
using the following command:
```
autopep8 -i -r --exclude $(sed -e 's|^|./|' -e 's|/$||' .ci/flake8_blacklist.txt | paste -sd,) --select E201,E202 .
```
Fix E127 errors introduced by previous commit
using:
```
autopep8 -i -r --exclude $(sed -e 's|^|./|' -e 's|/$||' .ci/flake8_blacklist.txt | paste -sd,) --select E127 .
```

@nsoranzo nsoranzo force-pushed the nsoranzo:fix_E201_E202 branch from 7552aad to a8d7b2d Aug 17, 2017

@nsoranzo

This comment has been minimized.

Copy link
Member Author

commented Aug 17, 2017

@mvdbeek Rebased.

@jmchilton jmchilton merged commit a85ee59 into galaxyproject:dev Aug 17, 2017

4 of 6 checks passed

api test Test started.
Details
framework test Test started.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
integration test Build finished. 44 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
toolshed test Build finished. 579 tests run, 0 skipped, 0 failed.
Details
@jmchilton

This comment has been minimized.

Copy link
Member

commented Aug 17, 2017

I'm definitely going to grow to hate you @nsoranzo as I have to rebase thousands of lines against this and this will make tracking git commit history for everything I've ever touched more difficult - but right now when we are still friends and since I don't hate you yet I thank you for doing this and congratulate you on moving ahead of me in the rankings.

@nsoranzo

This comment has been minimized.

Copy link
Member Author

commented Aug 17, 2017

Thanks @jmchilton ❤️
And I'm ahead of you only for deletions...

@nsoranzo nsoranzo deleted the nsoranzo:fix_E201_E202 branch Aug 17, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Aug 17, 2017

Add autopep8 script to help rebasing branches after galaxyproject#4440.
Fixes a few more PEP8 issues as part of the autopep8'ing as well.

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Aug 17, 2017

Add autopep8 script to help rebasing branches after galaxyproject#4440.
Fixes a few more PEP8 issues as part of the autopep8'ing as well.

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Aug 17, 2017

Add autopep8 script to help rebasing branches after galaxyproject#4440.
Fixes a few more PEP8 issues as part of the autopep8'ing as well.

dannon added a commit that referenced this pull request Aug 17, 2017

Merge pull request #4447 from jmchilton/autopep8
Add autopep8 script to help rebasing branches after #4440.
@jmchilton

This comment has been minimized.

Copy link
Member

commented Aug 18, 2017

If anyone else is in the position of having to rebase large branches against this change - I think have found a winning strategy - I'm half way through rebasing my thirty or so commits of the CWL branch against it using this approach:

# Create a backup of your branch pre-rebase and push to github.
git checkout -b my_branch_pre_rebase && git push origin my_branch_pre_rebase && git checkout my_branch

# Do a normal rebase against the last commit before the huge white space change
# this was we can be sure most of what we are rebasing against is just white space fixes
git rebase -i 008a1d849c4eb86ab52fd032ae611e4844c3cf09

# Do a rebase against just the whitespace changes
git rebase -i 68e8abc89aba1263f82c643d1bde89da53d70bc9

# For rebase will stop at each commit with a conflict, review -
# ideally what you see you want is the branch content but with white
# space changes - just verify this visually and quickly.
git diff

# Then checkout your branch content and apply whitespace fixes to it
# add it back in and continue the rebase
git checkout --theirs lib test && .ci/autopep8.sh && git add -u lib test && git commit && git rebase --continue

# After it is finally done - push to github and view changes - if everything is okay continue to rebase
# against latest dev.
git rebase -i upstream/dev
@peterjc

This comment has been minimized.

Copy link
Contributor

commented Aug 18, 2017

Its great to see this merged 🥇

Did anything need updating in the TravisCI / Tox settings to now enforce flake8's E201 and E202 checks?

(I've been looking at the files and /.ci/ folders and didn't find where the ignore/select list is done)

@martenson

This comment has been minimized.

Copy link
Member

commented Aug 18, 2017

@jmchilton already rebased my stuff yesterday but thanks for the guide

@nsoranzo

This comment has been minimized.

Copy link
Member Author

commented Aug 18, 2017

@peterjc The changes are in setup.cfg .

@VJalili

This comment has been minimized.

Copy link
Member

commented Aug 18, 2017

Finally I can enable pycharm code formatting :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.