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

[Suggestion] enable Prettier on this repository. #84

Closed
scalvert opened this issue May 11, 2020 · 8 comments · Fixed by #442
Closed

[Suggestion] enable Prettier on this repository. #84

scalvert opened this issue May 11, 2020 · 8 comments · Fixed by #442

Comments

@scalvert
Copy link
Contributor

After contributing to this repository, I noticed that Prettier isn't enabled. I'm a firm believer in adhering to the standards set forth by repository owners, but also see the value in standardizing on format across the community.

I was wondering if there'd be any appetite for enabling Prettier on this repo? I'd be willing to do the work to enable it and format all files.

@platinumazure
Copy link
Owner

Hi @scalvert, thanks for the issue.

I've never used Prettier before and I do like the style I've set up for the most part, so this would need to be something I would need to review carefully. I can see some value in standardizing on style, but my take on this has always been "let people write code how they want and let the linter fix it to enforce repository standards".

I'll consider it. If you can cite any specific style changes that you think would be beneficial, feel free to suggest them and who knows, maybe we can apply them even if we don't go for Prettier.

@scalvert
Copy link
Contributor Author

I appreciate your take on this, and it's entirely your prerogative to format the code how you wish. Most of the repositories I either work on or contribute to almost exclusively use Prettier, and I've found it to be a breath of fresh air!

In case you're curious, and in the Prettier site's own words:

By far the biggest reason for adopting Prettier is to stop all the on-going debates over styles. It is generally accepted that having a common style guide is valuable for a project and team but getting there is a very painful and unrewarding process. People get very emotional around particular ways of writing code and nobody likes spending time writing and receiving nits.

ref: https://prettier.io/docs/en/why-prettier.html

I think it's hugely powerful to have a predictable format from repo to repo. At this point, the only repo I contribute to (occasionally) that doesn't use Prettier is QUnit, and that's largely for historical reasons.

@platinumazure
Copy link
Owner

@scalvert I've taken a look and the Prettier style seems pretty close to what I like. I'm willing to review a PR and see what it looks like in this code base. I do not guarantee that I will merge the PR.

A few things I don't want to change:

  • Indent: 4 spaces, no tabs (I just can't read 2 space indentation, it makes the code feel too condensed for my liking)
  • Semicolons: Must be present (no ASI)
  • Print width: No higher than 80 characters

Other thoughts:

  • Quote properties: Let's look at "as-needed", but I may request that we switch to "preserve" depending on what happens with "as-needed"
  • Trailing commas: "es5" is reasonable in first pass, but we could maybe do "all" if this happens after the major release (where I will drop support for Node 6 and Node 8).
  • Arrow function parens: I'd like to see what "always" will do-- might result in more changes than I would like. I may request "avoid" instead.

Otherwise, please look at the ESLint config for guidance, or use the default options and/or common sense.

Thanks for offering to explore this!

@scalvert
Copy link
Contributor Author

So typically the only thing I usually enforce in prettier are the following:

module.exports = {
  singleQuote: true,
  trailingComma: 'es5',
  printWidth: 80,
};

Everything else is usually left as the default. I'd opt for that, as the entire point of Prettier is to not have these discussions anymore :D

@platinumazure
Copy link
Owner

Seems we are having the discussion anyway...

If you don't agree with how I'd like to implement Prettier, then we can just close this. My current style is also enforceable pretty well as it is. I make a point of not making nitpicks in code reviews that I can't enforce via linting. So as long as people run the linter before opening a PR, then PR reviews should already be free of style discussion and nitpicking, generally speaking.

It doesn't escape my notice that there were no discussions of style in this repo, until you brought up Prettier. You're using the "let's avoid these discussions" as justification, but you started the discussion in the first place. I considered your suggestion and gave my feedback on how I would like to do it, but you're pushing back and insisting on a specific style-- while still insisting that doing things your way would stop this discussion which you started.

I engaged in this discussion in good faith, and even took a look at Prettier which I hadn't had a chance to do before. I'm not sure that you are discussing with good faith at this point. It kind of feels like you are trying to strong-arm me into adopting your specific style (not even the Prettier default-- I see your singleQuote: true), rather than providing a suggestion for a useful tool that could help contributors. If you are trying to discuss in good faith, I appreciate it, but I just wanted to let you know how this feels from my side.

If you aren't willing to work on a PR that adds Prettier according to my preferences above, that's completely okay and we can close this. I understand that any contributions you make are completely voluntary and I appreciate it.

@scalvert
Copy link
Contributor Author

Well that escalated quickly! See how passionate people get about code formatting!!

I’m totally fine to close this issue if it’ll cause too much trouble. Sorry for the suggestion!

@platinumazure
Copy link
Owner

I appreciate that you brought it up and convinced me to explore Prettier. I hadn't looked at it before. I just thought it seemed a little hypocritical of you to criticize me for deviating from defaults (4-space indentation) when you also deviated from defaults (prefer single quotes). I'll consider implementing Prettier, but it's clear I should just do it myself rather than going back and forth with you about it when you and I could both be using our time more wisely.

Thanks again for the discussion!

@scalvert
Copy link
Contributor Author

Sounds good. Looking forward to the improved consistency. I think it’ll make it much easier to contribute to this repo! Let me know how I can help.

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

Successfully merging a pull request may close this issue.

2 participants