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

Flake8: remove E306 and F523 #4572

Merged
merged 1 commit into from May 14, 2020

Conversation

noviluni
Copy link
Member

It seems that the last released version of pytest-flake8 fixes the issue causing the Travis pipeline to fail. However, I've seen that it's raising some new cases.

@codecov
Copy link

codecov bot commented May 13, 2020

Codecov Report

Merging #4572 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4572   +/-   ##
=======================================
  Coverage   84.55%   84.55%           
=======================================
  Files         164      164           
  Lines        9923     9923           
  Branches     1476     1476           
=======================================
  Hits         8390     8390           
  Misses       1266     1266           
  Partials      267      267           

@elacuesta
Copy link
Member

Hey @noviluni, great work on the upstream fix, thank you very much!
I'm working on removing E741 at #4558, it seems like these didn't appear before because they are within for loops. Do you think you can remove E741 additions in this PR? That would leave only four ocurrences of E306/F523, do you do you think you could edit the offending files instead?

@elacuesta elacuesta mentioned this pull request May 13, 2020
@noviluni
Copy link
Member Author

sure! 👍 @elacuesta

@noviluni
Copy link
Member Author

Hi @elacuesta, could you take a look? Is this what you requested?

@@ -19,7 +19,7 @@ def _isiterable(possible_iterator):


def _fname(f):
return "%s.%s".format(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guilty of this one, until now I hadn't even realized about its oddity.

Copy link
Member

@elacuesta elacuesta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Precisely! Thanks again 💪

@elacuesta elacuesta changed the title add missed flake8 rules to pytest.ini Flake8: remove E306 and F523 May 13, 2020
@elacuesta
Copy link
Member

Travis is failing with 429, perhaps we are triggering too many builds? 😂

@elacuesta elacuesta merged commit cfe4bf7 into scrapy:master May 14, 2020
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants