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

Apply general spring style formatting 🌻 #860

Merged
merged 5 commits into from
Apr 1, 2019
Merged

Apply general spring style formatting 🌻 #860

merged 5 commits into from
Apr 1, 2019

Conversation

bonjourmauko
Copy link
Member

@bonjourmauko bonjourmauko commented Mar 29, 2019

Spring is here

  • Apply flake8-bugbear recommendations

@bonjourmauko bonjourmauko added level:starter Suited for beginners and new contributors kind:improvement Refactoring and code cleanup in progress labels Mar 29, 2019
Copy link
Contributor

@Morendil Morendil left a comment

Choose a reason for hiding this comment

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

I'm 👍 on adding bugbear and following its recommendations, 👎 on "general formatting changes" which make this a very noisy PR (lots of conflict management to be expected with other long-running branches) and contain changes not mandated by our official linting checks.

@bonjourmauko
Copy link
Member Author

bonjourmauko commented Mar 29, 2019

Hi @Morendil !

I'm 👍 on adding bugbear and following its recommendations

I'll split in two different pull requests.

Make this a very noisy PR

What could we do to solve that issue? Split in several different pull requests? Or maybe it is just not worth the effort and we should live with it.

contain changes not mandated by our official linting checks

I did a catch-all to go fast so there maybe some errors.

My objective is to:

  • Correct no spaces around = : this is not enforceable and is not PEP endorsed, but it is the style used majoritarily by this repo.

  • Correct hang-closing indentation : here again, auto-formatter do not enforce rules 100% of the time.

I'm OK removing all the rest.

Those changes, AFAIK, are the ones we would like to enforce, will our linting checks allow us to (see #706 and openfisca/openfisca-france#1060).

@Morendil
Copy link
Contributor

What could we do to solve that issue?

My best suggestion is to make the changes when you touch these files for some other reason. This spreads the changes over time and keeps them correlated with functional changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:improvement Refactoring and code cleanup level:starter Suited for beginners and new contributors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants