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

Enforce black code formatting #851

Open
tristanlatr opened this issue Dec 9, 2024 · 3 comments
Open

Enforce black code formatting #851

tristanlatr opened this issue Dec 9, 2024 · 3 comments

Comments

@tristanlatr
Copy link
Contributor

I'm opening this issue to talk about how should we enforce code formatting (not whether we should enforce it).

We basically have two options:

I don't have a strong opinion, so I'de like to request yours @adiroiban @glyph, thanks

@glyph
Copy link
Member

glyph commented Dec 9, 2024

Option 1 is obviously simpler and quicker, and, all things being equal, I tend to prefer it.

However, the major reason not to do things that way is to avoid merge conflicts with work in progress.

I see we have 17 PRs outstanding — most of them by you :). If those PRs are amenable to a simple rebase against the reformat, and it won't create big problems, then let's just do it that way.

IIRC the way we dealt with this on Twisted itself was to have a script that would rebase commits with blackened versions of themselves, which mostly addressed the problem. I don't remember what happened to that script though, it's been a while.

So: if the conflicts are a problem, using darker until we have fewer outstanding PRs and then doing the all-at-once thing should address that. Otherwise, let's format all at once and move on as quickly as we can.

Thanks for asking!

@tristanlatr
Copy link
Contributor Author

Hello @glyph and thanks for your input, I did a little digging of https://github.com/twisted/twisted/pull/1380/files and https://github.com/twisted/twisted/pull/1381/files but did not find this script you are talking about...

@glyph
Copy link
Member

glyph commented Dec 9, 2024

Hello @glyph and thanks for your input, I did a little digging of https://github.com/twisted/twisted/pull/1380/files and https://github.com/twisted/twisted/pull/1381/files but did not find this script you are talking about...

I think maybe we just did it manually for all PRs, with a git rebase --exec=... one-liner like this? https://stackoverflow.com/a/73982786/13564

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants