Add prettier config #176
Add prettier config #176
Conversation
✅ Deploy Preview for webkit-jetstream-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Generally LGTM with some nits and these high-level questions:
I didn't implement the dual prettier + eslint approach
Since I'm not familiar with how it's done for Speedometer: Does this mean first running prettier and then (only a subset of) eslint? Keeping it simpler sounds good to me.
I personally introduced a few bugs due to the no-braces for single block statements, so I'd be more than happy to loosen this requirement and go with the prettier defaults for sake of simplicity.
- Single block statements are on the same line
- Braces are required to force a new line for block statements
Strong +1, I also dislike no-braces for single block statements, especially in the combination with blocks with braces, e.g.,
if (cond)
stmt1;
else {
stmt2;
stmt3;
}
Question 1: Do we plan on having a "big formatting" CL at some point? Or iteratively (per-file or even more fine grained)?
Question 2: Do we plan on having a pre-commit hook in the future? If yes, will it always format everything? (Which would be fine if everything is already in a formatted state, otherwise not so much.)
|
Q1: yes, I'd plan to make a big format PR and then – but not touching the old workloads which have code inlined in benchmark.js Q2: We can try this (for speedometer we only added a CQ check, because this was too slow so we didn't do it), I'd probably be in favour of having having a direct push / commit hook |
|
@kmiller68 how strongly do you feel about the braces? I find them rather error prone – I only know WebKit who does that personally so very little day-to-day exposure on this. |
|
I don't mind braces (were I writing a project from scratch I'd probably expect them). I think JetStream tends to follow WebKit style overall but I personally don't care too much. I'll probably mess up a lot and elide the braces so as long as something complains when I upload the PR I'm fine with whatever. Let me check if other folks internally care. |
|
Not feeling strongly about this either, but I'd definitely add an auto-formatter for the harness code so we wouldn't have to really deal with this manually. |
This PR copies over the prettier config from Speedometer.
Note that I didn't implement the dual prettier + eslint approach, I always found that annoying (we need to duplicate the ignore files) and slow.
In Speedometer this was introduced to more closely adhere to WebKit's style guide which I always found a bit problematic. I personally introduced a few bugs due to the no-braces for single block statements, so I'd be more than happy to loosen this requirement and go with the prettier defaults for sake of simplicity.
Beyond that I presonally don't care too much about code formatting, if there is an easy way to get closer to WebKit I'm fine.