Make code base compliant with PEP-8 E-261. #5144

adnrs96 · 2017-05-31T23:16:03Z

This PR will track the commits which will eventually lead our code base to 100% compliance with the PEP-8 E-261. According to E-261 two spaces must be used before making an inline comment.
This contains changes which might cause merge conflicts but won't require manual intervention while rebasing. Automerge can deal with them. Also after this some 26 files are left before we can enable this check in linters.

smarx · 2017-05-31T23:16:04Z

Automated message from Dropbox CLA bot

@adnrs96, it looks like you've already signed the Dropbox CLA. Thanks!

timabbott · 2017-06-01T00:13:03Z

Merged, thanks @adnrs96! Leaving this open so you can add more commits.

What are the PRs conflicting with the big files left (e.g. models.py)? It might make sense to just convert those and suffer a few conflicts.

adnrs96 · 2017-06-01T07:06:05Z

These are the totality of the remaining files which needs to be cleaned. All of these will be causing merge conflicts to any on of the associated PR. I now feel that I could have augmented the scanning tool to tell exactly with with PR is the conflict going to occur. I guess we should do a second round with files which will cause conflicts to PR's (But PR's are small enough to rebase manually without much effort. eg 'zproject/dev_settings.py PR#1747(+3-0) PR#4387(+3-0)).

timabbott · 2017-06-02T05:12:13Z

I think some of the conflicts PRs are now cleaned up / closed. I think you can definitely proceed with zerver/tornado/event_queue.py; that conflict is small and the file is big.

I think we should be able to clear out several more of the PRs that conflict here in the next week or so.

#3367 and #1747 already have a lot of conflicts and we can ignore for this purpose.

adnrs96 · 2017-06-04T11:18:25Z

@timabbott Did some more clean up. Still there are around 20 of files from that list pending.
I have added the last commit which is focused on enabling the linter to check for pep-E261 violations by default with an ignore list of those 20 files. This should ensure no new violations to this rule could get onto master. (Observed some new violations being added recently).

timabbott · 2017-06-04T16:19:07Z

Great, merged! I think shortly after 1.6 we'll be able to merge several of the conflicting PRs.

zulipbot · 2017-06-05T17:37:22Z

Heads up @adnrs96, we just merged some commits (latest: 5fe7ed8) that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

adnrs96 · 2017-07-09T00:26:35Z

None of these are merge conflict free, I tried to order them as models.py, bugdown/__init__.py and then files which I felt had minimum number of changes. I haven't pushed all the files (I can do that I wanted but all of them will cause merge conflicts). Files for which commits are not there currently in this PR

'zerver/tests/test_bugdown.py',
        'zerver/tests/test_events.py',
        'zerver/tests/test_messages.py',
        'zerver/tests/test_narrow.py',
        'zerver/tests/test_realm.py',
        'zerver/tests/test_signup.py',
        'zerver/tests/test_subs.py',
        'zerver/tests/test_upload.py',

Will post the conflict report after this in separate message.

(My theory for low number of changes was that conflicts would probably be easy to solve if we end up merging them)

adnrs96 · 2017-07-09T00:31:48Z

tools/run-dev.py PR#5224(+6-6) PR#4880(+1-1) PR#4973(+11-4)
zerver/lib/bugdown/init.py PR#5633(+19-5) PR#4110(+2-1) PR#4132(+5-2) PR#2167(+9-1) PR#5253(+27-0) PR#2200(+18-1) PR#2248(+1-1) PR#2170(+20-4) PR#3923(+5-7) PR#4973(+16-2) PR#1935(+2-1) PR#5521(+1-0) PR#5594(+30-11)
zerver/models.py PR#5633(+25-0) PR#4110(+4-4) PR#5649(+7-1) PR#5658(+19-9) PR#5665(+3-0) PR#5723(+1-0) PR#5720(+2-2) PR#1122(+36-0) PR#5224(+2-3) PR#5141(+12-2) PR#1179(+14-8) PR#3317(+7-0) PR#5367(+2-0) PR#1325(+42-0) PR#5341(+3-0) PR#3382(+2-0) PR#1339(+20-0) PR#4451(+36-0) PR#5525(+14-0) PR#1432(+3-0) PR#5573(+2-0) PR#850(+1-0)
zerver/tests/test_bugdown.py PR#5633(+33-0) PR#4110(+20-0) PR#4132(+1-1) PR#2170(+11-0) PR#3923(+2-2) PR#4973(+34-0) PR#5594(+8-0)
zerver/tests/test_events.py PR#5669(+2-0) PR#4162(+97-79) PR#5240(+84-2) PR#5367(+2-0) PR#3382(+1-0) PR#5023(+55-25) PR#5573(+1-0) PR#4065(+67-0)
zerver/tests/test_messages.py PR#5633(+1-1) PR#4110(+40-0) PR#3346(+20-20) PR#1325(+38-2) PR#1339(+38-2)
zerver/tests/test_narrow.py PR#3367(+4-4) PR#3051(+74-0)
zerver/tests/test_realm.py PR#5658(+4-4) PR#5367(+1-0) PR#3382(+1-0) PR#4586(+7-5)
zerver/tests/test_signup.py PR#5633(+1-1) PR#5656(+2-2) PR#5658(+7-12) PR#5720(+2-2) PR#1747(+78-7) PR#4393(+12-0) PR#5532(+0-4)
zerver/tests/test_subs.py PR#5633(+3-3) PR#1122(+17-1) PR#1747(+2-2) PR#749(+37-1) PR#3367(+1-1) PR#2984(+7-1)
zerver/tests/test_upload.py PR#3706(+15-9) PR#4387(+18-1) PR#4973(+32-0)
zerver/tornado/socket.py PR#3202(+55-13)
zerver/tornado/websocket_client.py PR#3202(+49-54)
zerver/worker/queue_processors.py PR#5633(+9-0) PR#5649(+1-0) PR#5720(+2-2) PR#3367(+1-1) PR#5422(+4-1)
zilencer/management/commands/populate_db.py PR#5633(+6-0) PR#1122(+7-5) PR#4946(+18-7) PR#5525(+14-0)
zproject/dev_settings.py PR#1747(+3-0) PR#4387(+3-0)
zproject/prod_settings_template.py PR#5224(+0-8) PR#4387(+6-7) PR#4973(+4-0)
zproject/settings.py PR#5669(+2-0) PR#5720(+2-1) PR#5224(+2-3) PR#2172(+1-0) PR#4767(+1-0) PR#5300(+1-0) PR#5445(+0-5) PR#4305(+10-2) PR#1747(+13-0) PR#5345(+1-0) PR#4387(+11-0) PR#3367(+6-3) PR#4393(+5-1) PR#5239(+1-0) PR#3397(+10-0) PR#4973(+3-0) PR#4986(+3-0) PR#388(+1-0) PR#3227(+2-1)

adnrs96 · 2017-07-09T00:34:58Z

@timabbott Please take a look.
(Sorry for the super delay on this)

timabbott · 2017-07-11T19:00:34Z

I looked through these, and they all seemed pretty low-risk except models.py and bugdown, which I think we just need to merge at this point. Merged, thanks @adnrs96!

Looking at the remaining changes, to test files, I think it's probably safe to push those through. The PRs they'll conflict with all basically just add new tests, so there's unlikely to be interesting merge conflicts.

So I think we can finish this off!

adnrs96 · 2017-07-11T19:05:09Z

Sure, I will update the PR.

zulipbot · 2017-07-11T19:40:44Z

Heads up @adnrs96, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

adnrs96 · 2017-07-11T20:07:36Z

@timabbott Just updated this. After this pep8 E261 migrations will come to an end 🎉

adnrs96 · 2017-07-11T22:10:02Z

hmm, I left out a glitch earlier and that's why the tests failed but fixed it.
Probably shouldn't be a problem now

timabbott · 2017-07-11T22:19:05Z

Merged, thanks @adnrs96! It's awesome to have this big migration complete :)

zulipbot added the needs review label May 31, 2017

timabbott mentioned this pull request Jun 2, 2017

(wip) Add Topic table #1289

Closed

timabbott mentioned this pull request Jun 2, 2017

i18n: Add linter to ensure handlebars strings are line-wrapped properly #4280

Closed

adnrs96 force-pushed the pep-8-E261 branch from 87da96b to 58e02dd Compare June 4, 2017 11:14

adnrs96 force-pushed the pep-8-E261 branch 2 times, most recently from 3054847 to 99edb94 Compare June 4, 2017 11:22

adnrs96 force-pushed the pep-8-E261 branch from 99edb94 to 35f414f Compare July 9, 2017 00:23

adnrs96 force-pushed the pep-8-E261 branch from 35f414f to 449d513 Compare July 9, 2017 00:28

adnrs96 added 6 commits July 12, 2017 01:33

pep8: Add compliance with rule E261 to tests/test_bugdown.py.

7dcc62f

pep8: Add compliance with rule E261 to tests/test_events.py.

d573db3

pep8: Add compliance with rule E261 to tests/test_messages.py.

1184012

pep8: Add compliance with rule E261 to tests/test_narrow.py.

d43a885

pep8: Add compliance with rule E261 to tests/test_realm.py.

2f1f6d8

pep8: Add compliance with rule E261 to tests/test_signup.py.

6ed5a40

adnrs96 force-pushed the pep-8-E261 branch from 449d513 to 867fded Compare July 11, 2017 20:05

pep8: Add compliance with rule E261 to tests/test_subs.py.

9493b0a

adnrs96 added 2 commits July 12, 2017 03:36

pep8: Add compliance with rule E261 to tests/test_upload.py.

2aaeb88

linter/pep8: Remove code infra to ignore the E261 violations.

765ce68

adnrs96 force-pushed the pep-8-E261 branch from 867fded to 765ce68 Compare July 11, 2017 22:06

timabbott closed this Jul 11, 2017

zulipbot removed the needs review label Jul 11, 2017

adnrs96 deleted the pep-8-E261 branch July 11, 2017 22:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make code base compliant with PEP-8 E-261. #5144

Make code base compliant with PEP-8 E-261. #5144

adnrs96 commented May 31, 2017

smarx commented May 31, 2017

timabbott commented Jun 1, 2017

adnrs96 commented Jun 1, 2017

timabbott commented Jun 2, 2017

adnrs96 commented Jun 4, 2017

timabbott commented Jun 4, 2017

zulipbot commented Jun 5, 2017

adnrs96 commented Jul 9, 2017 •

edited

Loading

adnrs96 commented Jul 9, 2017

adnrs96 commented Jul 9, 2017

timabbott commented Jul 11, 2017

adnrs96 commented Jul 11, 2017

zulipbot commented Jul 11, 2017

adnrs96 commented Jul 11, 2017

adnrs96 commented Jul 11, 2017 •

edited

Loading

timabbott commented Jul 11, 2017

Make code base compliant with PEP-8 E-261. #5144

Make code base compliant with PEP-8 E-261. #5144

Conversation

adnrs96 commented May 31, 2017

smarx commented May 31, 2017

timabbott commented Jun 1, 2017

adnrs96 commented Jun 1, 2017

timabbott commented Jun 2, 2017

adnrs96 commented Jun 4, 2017

timabbott commented Jun 4, 2017

zulipbot commented Jun 5, 2017

adnrs96 commented Jul 9, 2017 • edited Loading

adnrs96 commented Jul 9, 2017

adnrs96 commented Jul 9, 2017

timabbott commented Jul 11, 2017

adnrs96 commented Jul 11, 2017

zulipbot commented Jul 11, 2017

adnrs96 commented Jul 11, 2017

adnrs96 commented Jul 11, 2017 • edited Loading

timabbott commented Jul 11, 2017

adnrs96 commented Jul 9, 2017 •

edited

Loading

adnrs96 commented Jul 11, 2017 •

edited

Loading