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

Add support for autoformatting using yapf #13528

Merged
merged 3 commits into from
Feb 25, 2022
Merged

Conversation

algmyr
Copy link
Contributor

@algmyr algmyr commented Feb 12, 2022

I have my personal ideological issues with Black's opinionated formatting, so I made the small necessary change to add support for yapf as a formatter. I'm all for having many options for this. :)

Thankfully the option was already general enough that the addition was very simple, so good design work there!

@algmyr
Copy link
Contributor Author

algmyr commented Feb 12, 2022

I suspect the formatting check is a false positive? Using single quotes matches the existing code.

@bollwyvl
Copy link
Contributor

I suspect the formatting check is a false positive?

darken's is set up such that all new changes do conform to this repo's target of conforming to a stock black configuration... in this case, double quotes.

ideally, we'd have a bot that just applied these changes, rather than breaking the build: i suppose.

@algmyr
Copy link
Contributor Author

algmyr commented Feb 12, 2022

darken's is set up such that all new changes do conform to this repo's target of conforming to a stock black configuration... in this case, double quotes.

ideally, we'd have a bot that just applied these changes, rather than breaking the build: i suppose.

So what do you prefer? Make darken happy but be inconsistent or make darken sad but be consistent? My own 2c would be to keep it consistent and the quote style change can be done at a later stage in some larger scale refactoring.

@bollwyvl
Copy link
Contributor

says right in the log:

 Changes need auto-formatting. Run:
    darker -r 60625f241f298b5039cb2debc365db38aa7bb522
then commit and push changes to fix.

@algmyr
Copy link
Contributor Author

algmyr commented Feb 12, 2022

darker -r 60625f2

darker is broken locally for me akaihola/darker#264 and I'm not so keen on setting up a venv just for this, I'll just manually fix the one line that it complains about.

@Carreau Carreau added this to the 8.1 milestone Feb 25, 2022
@Carreau
Copy link
Member

Carreau commented Feb 25, 2022

Thanks, sorry about the issue with formatting.

@Carreau Carreau merged commit 935dedb into ipython:master Feb 25, 2022
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.

3 participants