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

Consideration for Prettier #1014

Closed
jiji14 opened this issue Oct 16, 2023 · 11 comments
Closed

Consideration for Prettier #1014

jiji14 opened this issue Oct 16, 2023 · 11 comments

Comments

@jiji14
Copy link

jiji14 commented Oct 16, 2023

Consideration for Prettier

While rewriting Angular services, I noticed mixed code styles. I believe it would be beneficial to set up Prettier configuration in our project.

what is Prettier ?

Prettier is an opinionated code formatter. It removes all original styling and ensures that all outputted code conforms to a consistent style.

why Prettier ?

It is generally accepted that having a common style guide is valuable for a project and team.
Prettier assists in writing clean code.

Examples:

tabWidth: 4 vs tabWidth: 2
singleQuote: true vs singleQuote: false

Resources

You can learn more about Prettier here: Prettier
Additionally, this article explains why coding style is important: Why Coding Style Matters

@jiji14 jiji14 changed the title Consideration of Prettier Consideration for Prettier Oct 16, 2023
@JGreenlee
Copy link

I briefly considered this when we started the rewrite. I saw that we already have an .editorconfig file, which I believe is supposed to be a less-opinionated solution than Prettier. However, I'm not sure if VSCode is even picking up on the .editorconfig file.

I would argue that although the old Angular code was pretty messy, the code we have rewritten so far is mostly uniform in style. For example I have been consistent about using 2-space indents, and fairly consistent about using single quotes rather than double quotes.

I can see the value of Prettier, but our team is not that big. On the other hand, it is an open source project and we do want to encourage contributions from the community, so I can see it both ways.

@jiji14
Copy link
Author

jiji14 commented Oct 16, 2023

I briefly considered this when we started the rewrite. I saw that we already have an .editorconfig file, which I believe is supposed to be a less-opinionated solution than Prettier. However, I'm not sure if VSCode is even picking up on the .editorconfig file.

I would argue that although the old Angular code was pretty messy, the code we have rewritten so far is mostly uniform in style. For example I have been consistent about using 2-space indents, and fairly consistent about using single quotes rather than double quotes.

I can see the value of Prettier, but our team is not that big. On the other hand, it is an open source project and we do want to encourage contributions from the community, so I can see it both ways.

Thanks for your opinion! Given that our project is open source, I think you are right. I will close the issue.

@jiji14 jiji14 closed this as completed Oct 16, 2023
@JGreenlee
Copy link

I actually meant that the project being open-source is one reason we would consider using Prettier. I think the more people are working on a project, the more important it is to unify code styles. We don't have a huge dedicated team, but theoretically anyone from the community could contribute. If we saw an uptick in community contributions in the future, we might benefit from a stricter code style.
So I think we should get @shankari's opinion before proceeding.

@jiji14 jiji14 reopened this Oct 16, 2023
@jiji14
Copy link
Author

jiji14 commented Oct 16, 2023

I actually meant that the project being open-source is one reason we would consider using Prettier. I think the more people are working on a project, the more important it is to unify code styles. We don't have a huge dedicated team, but theoretically anyone from the community could contribute. If we saw an uptick in community contributions in the future, we might benefit from a stricter code style.
So I think we should get @shankari's opinion before proceeding.

My apologies for the misunderstanding. I have now reopened the issue.

@shankari
Copy link
Contributor

@JGreenlee @jiji14 I think you should go ahead and set this up. Even considering only internal contributions, we have interns who are here for short periods of time and it is good to have that continuity of code style. It will at least cut down on the inadvertent whitespace changes issue and make pull requests cleaner.

I would suggest:

  • after you set up prettier, do one big PR that converts all the code into the desired format, so that is the "cleanup PR"
  • enable prettier in GitHub actions so we can check it for every subsequent PR
  • I will make sure to only merge PRs that have prettier passing.

@JGreenlee
Copy link

My suggestions / the style I have generally been following the past few months:

  • Max line width 100 characters
  • Tab width 2 spaces
  • Single quotes
  • Trailing commas
  • Bracket spacing
  • Arrow brackets on same line in JSX
  • \n at end of file

These are all open to discussion if anyone would like to express a different preference

@jiji14
Copy link
Author

jiji14 commented Oct 17, 2023

what do you think about adding semicolons at the ends of statements?

@JGreenlee
Copy link

I vote yes to semicolons

@shankari
Copy link
Contributor

I wrote C++ code every day for 8 years. semicolons all the way!

@the-bay-kay
Copy link

These all sound great to me! I think my only hesitation is using "arrowParens": "avoid". If we're making an arrow function without parameters, wouldn't we still need parens (e.g., const foo = () => {return 'bar'}; )? It's an edge case for sure, I'm not sure if that's a style inconsistency we'd want to consider.

The Prettier docs (link) suggest that avoiding parentheses can make type annotations and extra arguments tricky, which is another thing to consider. No parentheses would be a cleaner look, but having them may help us avoid some syntax hiccups!

@jiji14
Copy link
Author

jiji14 commented Oct 18, 2023

These all sound great to me! I think my only hesitation is using "arrowParens": "avoid". If we're making an arrow function without parameters, wouldn't we still need parens (e.g., const foo = () => {return 'bar'}; )? It's an edge case for sure, I'm not sure if that's a style inconsistency we'd want to consider.

The Prettier docs (link) suggest that avoiding parentheses can make type annotations and extra arguments tricky, which is another thing to consider. No parentheses would be a cleaner look, but having them may help us avoid some syntax hiccups!

I deleted "arrowParens": "avoid"! Thanks for your comment :)

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

No branches or pull requests

4 participants