web-platform-tests / rfcs

web-platform-tests RFCs
75 stars 63 forks source link

Add .prettierrc to WPT #129

Open marcoscaceres opened 1 year ago

marcoscaceres commented 1 year ago

Summary

Tests throughout WPT are inconsistently formatted, which makes reading and editing them somewhat tedious.

Details

It would be nice to add a .prettierrc file, to support tidying HTML, CSS, and JS files directly through code editors.

It would be nice to place some variation of following into the root of WPT.

{
  "endOfLine": "lf",
  "printWidth": 80,
  "tabWidth": 2,
  "trailingComma": "es5"
}

However, the proposal is NOT to reformat all the files. Just to have the prettier file there for folks that want to use it when writing new tests (or want to update badly formatted tests).

This is also nice for when WPT gets ingested in downstream projects, which have their own coding conventions. It means that tests written downstream can still be formatted with Prettier.

Risks

Some files shouldn't be linted (e.g., ref test).

Re-Formatting files changes usually changes them entirely, which affects the commit history. This can be a little annoying, because then folks need to look back over the commit history to see what/when things were changed (and who to blame).

Not everyone has prettier (and it's yet another dependency). However, projects like Gecko do use it so there is some precedent.

jgraham commented 1 year ago

So we can't use auto-formatting because we can't enforce that all downstream consumers (Blink, Gecko, etc.) have exactly the same versions of the same tools installed. We especially don't want to be in a situation where format A is required by some downstream project and slightly different format A' is required by the upstream CI, so that it's impossible to land patches in one place that meet the requirements of the other.

As you note, it's also unusually likely that tests require some specific formatting (although that on its own wouldn't be a blocker).

However this proposal is not that, and it seems plausible that we could check in some configuration files to allow people who are using these tools anyway to use them with wpt.

However, I think we'd still want to avoid gratuitous formatting changes (as you note), and try to keep/develop a culture of not nitpicking formatting in code reviews (i.e. I wouldn't want to see this kind of change cause multiple review cycles to be spent on "this code doesn't look exactly like it would have done if you'd been using [tool], therefore you have to change it")

Ms2ger commented 1 year ago

Adding this file is likely to cause people to misunderstand it as defining a preferred / normative coding style, which I don't think we should create. I feel like it would harm more than it would help.

marcoscaceres commented 1 year ago

However this proposal is not that, and it seems plausible that we could check in some configuration files to allow people who are using these tools anyway to use them with wpt.

Yeah, so I guess perhaps it could be on directory basis?... like, for instance, a working group might decide that they want their tests formatted in a particular way (i.e., include the .prettierrc file inside some-directory/).