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

Restyle repository to 80 columns. #323

Closed
wants to merge 3 commits into from

Conversation

MatthewHambley
Copy link
Collaborator

Simply bumping up the acceptable line width doesn't seem like a good way to tackle the problem of overly nested structures.

This change removes the special settings and restyles everything to 80 columns with exceptions where doing so would tie us in knots.

This change does not attempt to tackle any of the problems such as the powerful odour coming from all the mockery in unit tests.

@MatthewHambley MatthewHambley marked this pull request as ready for review July 10, 2024 10:09
@hiker
Copy link
Collaborator

hiker commented Jul 10, 2024

While I totally agree with the 80 column column width, can we wait with this? We are at least 4 PRs ahead of trunk (and I have been reformatting things to 80 columns :) ), so please let us check what kind of conflicts this creates for us. I assume this is done with black (or a similar tool??), so we could do this on our end as the PR after if we have too many conflicts?

@MatthewHambley
Copy link
Collaborator Author

I have no problem with pulling this change if it messes with what you are doing.

@hiker
Copy link
Collaborator

hiker commented Jul 12, 2024

It causes over 20 conflicts for us, not sure how much effort it would be to fix them :( Did you just run black (or any other tool) on the sources? If we can re-run whatever you did, I would prefer that. But if you have done it manually, then we might have to bite the bullet and resolve the conflicts :(
FWIW, we are currently trying to change our fork to point back to this repo, atm we are 'disconnected', and can't even easily submit PRs anymore

@hiker hiker mentioned this pull request Aug 21, 2024
@MatthewHambley
Copy link
Collaborator Author

This change is being made piecemeal as other changes are made.

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.

2 participants