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

Consider updating automated code formatting tools' rules #924

Open
howard-e opened this issue Feb 14, 2024 · 6 comments
Open

Consider updating automated code formatting tools' rules #924

howard-e opened this issue Feb 14, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@howard-e
Copy link
Contributor

Currently, the auto-formatting tools uses prettier's default printWidth rule which defaults to 80. As the project grows and there are more deeply nested branches in the code, contributors have raised concerns or questions when it comes to readability (and perhaps ease of maintainability in some cases):

Generally, this discussion is a matter of "personal taste", but it seems the currently active contributors on this project align on increasing this number for practical readability reasons. Is there a reason to not to do this? What should it be increased to?

@howard-e howard-e added the enhancement New feature or request label Feb 14, 2024
@howard-e
Copy link
Contributor Author

Possibly related question - should tabWidth be considered in this discussion?

@stalgiag
Copy link
Contributor

Thanks for starting this discussion @howard-e ! I have wanted this for a while. We are deeply nested in many places and the default printWidth is frequently chopping up our code. I do think this is partially an invitation to nest our code less (smaller components, dedicated functions, not nesting conditionals where possible) but there is a lot of code that we won't be refactoring anytime soon that would have increased readability from a higher printWidth

@alflennik
Copy link
Contributor

Great idea!

Could I suggest we use tabWidth: 2 for the same reason? Which is also more idiomatic for JavaScript.

Also to improve git diffs I was wondering if we could use trailingComma: 'all'.

@jugglinmike
Copy link
Contributor

Thanks for starting this discussion, @howard-e.

My comment is an outlier among the three that you've referenced in that I was looking for guidance on the reduction of line length. Prettier's "printWidth" feature won't help us in that case as it tolerates string literals of any size. For what it's worth, I'm in favor of a strongly-enforced conventional maximum line length for the reasons that @stalgiag cited above. If the only hesitation to adopting such a policy is the existence of non-conforming code, I recommend making exceptions with in-line comments as necessary, both as a stop-gap for future contributions and as an explicit marker of intent for future refactoring.

@stalgiag
Copy link
Contributor

I think I agree with what @jugglinmike is proposing here as I do think there is useful accountability in the printWidth default. I would also say that this accountability is only useful if there is social reinforcement.

Going off @alflennik's, I would be in support of maintaining the default Prettier printWidth while changing the tabSize to '2'. This has become the most widely used tab size in modern JS projects and it would immediately improve readability across the entire project. This would then give us a fresh starting point from which to be more discerning about nesting depth while writing and reviewing code going forward. When reviewing, I currently worry that I will come off as nitpicky when I think to point out issues with readability when there are broad swaths of the project that have these issues. It might feel easier to establish a new standard that we all hold ourselves/each other to if we can see the advantages that come with improved readability.

@howard-e
Copy link
Contributor Author

howard-e commented Feb 26, 2024

A follow up on this is to create a PR which updates the project's default tabWidth to 2 to see how that addresses offending instances of odd formatting observed in the code.

Additional consideration for updating the printWidth to 100 may happen during that PR's review.

@howard-e howard-e changed the title Consider increasing printWidth Consider updating automated code formatting tools' rules Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants