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

Add .prettierrc to WPT #129

Open
marcoscaceres opened this issue Dec 5, 2022 · 3 comments
Open

Add .prettierrc to WPT #129

marcoscaceres opened this issue Dec 5, 2022 · 3 comments

Comments

@marcoscaceres
Copy link
Contributor

marcoscaceres commented Dec 5, 2022

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
Copy link
Contributor

jgraham commented Dec 5, 2022

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
Copy link
Contributor

Ms2ger commented Dec 5, 2022

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
Copy link
Contributor Author

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/).

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

No branches or pull requests

3 participants