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

Introduce shared, automatic code formatting #333

Open
tordans opened this issue Jan 6, 2022 · 4 comments
Open

Introduce shared, automatic code formatting #333

tordans opened this issue Jan 6, 2022 · 4 comments
Labels
question Further information is requested

Comments

@tordans
Copy link
Collaborator

tordans commented Jan 6, 2022

It would be a great help, if we had a shared code formatter config and documentation.

In #331 my Editor VS Code Reformatted all manner of things. I added a .prettierrc in 23da8f4 which made thinks better (https://prettier.io/docs/en/options.html).

  • Should we start using prettier?
    • Are the settings/options good?
  • Should re re-format all files once, or only once they are changed?
  • Should we use a different formatter instead?
  • Should we enforce the formatter via a Github Action as an error?
    • Or even apply the formatting changes via Github Actions as commit?
@matkoniecz
Copy link
Contributor

Should re re-format all files once

Yes, this prevents unexpected linter changes mixed with actual PR changes.

Should we enforce the formatter via a Github Action as an error?

That would make contributing harder (right now format is sufficiently clear that it is possible to contribute without running code)

Maybe have it as an info warning, but not blocking merge?

Or even apply the formatting changes via Github Actions as commit?
That seems nice if reasonable to setup and maintain.

@tyrasd tyrasd added the question Further information is requested label Jan 6, 2022
@tyrasd tyrasd added this to the v4 milestone Jan 6, 2022
@tyrasd
Copy link
Member

tyrasd commented Jan 6, 2022

In general, I would not be opposed to add such a thing. But to be honest, right out of the box, I don't particularly like the idea of of automated linting fixes. Also, I didn't experience any big issues with mixed-up indentations or other code formatting issues lately. Did you (//edit ah, I see #331 now… 🤔 )? If anything what sometimes feels slightly annoying is that the order of the different "sections" in the presets/fields are not always the same (e.g. sometimes the name is on top, sometimes on the bottom). A consistent order of these properties could make the files easier to read. Are there code linting tools which can do such a thing as well?

Since this would probably touch and change a lot of files, and it's low priority at the moment, I added this for the to do list of a next major release (working title v4) for now.

@1ec5
Copy link
Contributor

1ec5 commented Jan 6, 2022

The only potential issue I can think of is unreadable diffs caused by automatic reformatter like the one @tordans has installed. Messy diffs could make mistakes harder to spot in code review. GitHub has an “ignore whitespace” option, but I don’t think it handles cases like where an array gets flattened onto one line instead of one line per item.

@tordans
Copy link
Collaborator Author

tordans commented Jan 10, 2022

But to be honest, right out of the box, I don't particularly like the idea of of automated linting fixes.

Those are two things. Prettier only changes formatting and makes sure your frontend code is formatted the same way across developers.

Linting would also be great, but a different tool/config.

More at https://prettier.io/docs/en/comparison.html

Also, I didn't experience any big issues with mixed-up indentations or other code formatting issues lately.

Most users use the web editor from what I see based on the branch names ("patch-1"). But I am under the impression that the frontend development "culture" pretty much switched to automated code formatting on file save with Tools like VSCode.


Even if we don't use it or even enforce it, it would still be helpful to have for people like me who have this system set up.

@tyrasd tyrasd removed this from the v5 milestone Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants