Change our prettier rules for the default
Closed this issue · 13 comments
We have a bunch of overrides from the defaults for prettier.
Is there a strong reason to strictly stick to these?
If we follow the defaults we'll be "less weird".
Does "cargo culting" qualify as a "strong reason"?
Jokes aside, this was the result of trying to adhere to the "standards" that I could find in other projects, so it's a (temporary) mix of rules from the html devtools (the profiler at least) and the kinto-admin.
I'm open to discussion on those rules, as I don't have a clear preference myself (other than "let's get rid of JS syntax", but I'm not sure that's very constructive).
The nice thing about prettier is that we can change the rules and have them applied across the repository without any hassle \o/
What would you recommend @peterbe? Removing all the overrides?
The reason I care is because I see DD as a "thought leader" in our quest to churn out dashboards faster to align with our team goal. Personally I don't care that much, about the details. Especially since my Atom will just do all the labor for me.
The killer advantage to having coherent styles is that it's "easier on the eyes" when code you're reading looks familiar. Especially when it's a project you dip into temporarily or sporadically.
For example, personally I think 4-spaces is better for readability but it seems all code projects I get involved in uses 2-spaces so now, whenever I see 4-spaces code it just looks weird and jarring. Call that cargo culting if anything! :)
What I'm proposing is that we drop all overrides and go for Prettier's chosen defaults in hope that our code will look "as common as possible". This is what I'm trying personally to do with all my other JavaScript projects where I have the power to do so. It's also a subtle statement that the style actually doesn't matter; "no opinion here, only functionality matters".
Another option is to use meritocrazy rules and allow @magopian simply pick since he wrote most of the existing code in DD. Or we can have a vote within the team.
woah wait I'm not claiming meritocracy of anything ;) Also, as I said earlier, i don't care about the styles/rules. Also I do completely agree with the point of "looking as common as possible" (i can't recall why I took those overrides, so it mustn't have been a good reason ;)
Let's remove all the overrides! I mean, I'm voting for removing all of them (if there is a vote :P
Wanna get buy in from others? Or shall we just do it? It might cause big merge conflicts for all open PRs.
Sure we can see if @glasserc @Natim @leplatrem have some concerns (and maybe @mythmon @rehandalal and @andymikulski ?)
Also, we could merge all the PRs that can be merged before ;) (or deal with the aftermaths, i've done worse ;)
I'm generally pretty happy with auto code formatting. I'm not sure if Prettier is my favorite set of styles or my favorite tool, but that is sort of the point. It doesn't matter what the format is.
As far as dealing with PRs, wouldn't the merge conflicts be resolved by running Prettier with the new settings on the PR as well?
+1
I generally agree with @peterbe that having commonality makes code more legible and easier to dive into. so i'm a +1 on ditching overrides.
What I'm proposing is that we drop all overrides and go for Prettier's chosen defaults in hope that our code will look "as common as possible".
This sounds reasonable, +1
I agree that we should try to have our code look "as common as possible". However, I think we should emulate whatever Gecko, and especially Devtools, do/use, as they are currently the "Mozilla standard". So if the current set of Prettier overrides come from Devtools, then I think we should keep them.
@glasserc Do they use Prettier? If so, do they have a .prettierrc or something equivalent?
I'm not sure. I'm just responding based on Mago's comment that "it's a (temporary) mix of rules from the html devtools (the profiler at least)".