Skip to content
This repository has been archived by the owner on Dec 31, 2022. It is now read-only.

feat: use default value for quotes (double-quotes) #58

Merged
merged 4 commits into from
Jun 20, 2022
Merged

Conversation

kevgo
Copy link
Contributor

@kevgo kevgo commented Jun 18, 2022

Before I apply Prettier formatting consistently to all Ory repos (which will change a ton of files), I want to propose to remove the single-quote override for Prettier. Reasons:

  • Most languages we use here only work with double-quotes: Go, JSON, HTML, CSS, English.
  • Most JS tooling we use prefers double-quotes: Prettier, ESLint.
  • TS and YML can use either form but chose to deviate from the other languages we use. That feels elitist 🙂
  • I see consistency in code style across Ory as more important than conforming to (arbitrary) external style standards. Since we are a Go company, let's make our JS style match that of Go: no semi and double quotes for strings.
  • Most of the YML at Ory already uses double-quotes, let's not change that.
  • With the current config, TSX often has both types of quotes on a single line. That looks odd.

I am therefore proposing to remove the singleQuote: true override from the Ory-wide Prettier config. I think the benefit of removing pointless edge cases is worth the change. Curious what you think! No hard feelings if anybody sees it differently.

@kevgo kevgo requested review from aeneasr and zepatrik June 18, 2022 19:27
Copy link
Member

@zepatrik zepatrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to remind everyone here that

By far the biggest reason for adopting Prettier is to stop all the ongoing debates over styles.

Regardless, we never yet had a debate about this but just used these options for years now. Your arguments make sense, and I actually don't care about the specifics, as long as it is consistent.

So from my side it would be fine to update these values once and follow gofmt as much as possible is a good target.

semi: false,
singleQuote: true,
trailingComma: 'none'
trailingComma: "none"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would also be more consistent with go formatting.

Suggested change
trailingComma: "none"
trailingComma: "all",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent idea. This also minimizes diffs when adding or removing values from arrays.

@kevgo
Copy link
Contributor Author

kevgo commented Jun 20, 2022

Thanks, 100% agree! The larger roadmap towards ending style debates through Prettier is:

  1. remove configuration overrides that make code style less consistent across our code bases (this PR)
  2. ensure consistent formatting in all repos via CI (coming PRs)
  3. better support for people who are not familiar with Prettier

@kevgo kevgo merged commit 8cad2dd into master Jun 20, 2022
@kevgo kevgo deleted the kg-double-quotes branch June 20, 2022 16:06
@zepatrik
Copy link
Member

If you want to release this to npm, just create a release through github. But I guess you already figured that out 😉

@kevgo
Copy link
Contributor Author

kevgo commented Jun 20, 2022

That would have been my next question! I was assuming something like that from going through the release scripts and Git history but wanted to double-check with you. Thanks for reading my mind! 🙂

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

Successfully merging this pull request may close these issues.

2 participants