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 automatic code formatting via prettier #223

Closed
2 tasks
DanielMSchmidt opened this issue Aug 6, 2017 · 11 comments
Closed
2 tasks

Add automatic code formatting via prettier #223

DanielMSchmidt opened this issue Aug 6, 2017 · 11 comments

Comments

@DanielMSchmidt
Copy link
Contributor

Description

In order to improve consistency in the javascript part of the code base we could add a tool like prettier to enforce automatic formatting through the code base. This would help new contributors to contribute concise code as they won't have to adapt to a certain way of formatting.

Plan

  • add prettier formatting for the code base
  • add CI check that the code base is concise

What do you think?

@LeoNatan
Copy link
Contributor

LeoNatan commented Aug 9, 2017

I don’t like a linter enforcing some arbitrary code standards. Not that I write in JS, but still, conceptually, this looks like a source of inconvenience and annoyance.

@DanielMSchmidt
Copy link
Contributor Author

@LeoNatan Thanks for sharing your opinion in this, I really value your input on that topic.
Maybe my assumption that we want to have concise code was wrong in the first place. (For me this means no mix of tabs vs. spaces, single vs double quotes, spacing around brackets or not).

If we want concise code I don't see a point in not using prettier because it just abstracts that away from you and we might make conciseness as simple as one single command + CI check. If we don't have this goal we can close this issue.

For me personally the benefit of having a concise code base is that is way easier to read and it also IMHO allows you to spot errors more easy. We use it in my current company and we had no one who wanted to have this exact code style in the first place, but after two weeks of having it in the project there were just no more discussions around code styling. I'm a big fan of it, but I don't want to force it on this project 👍

@LeoNatan
Copy link
Contributor

I do want a good code, obviously, but this has to be weighted against the comfort of potential contributors (as well as first-party developers), and a balance JS to be stricken out. At the very least, if we decide to add such a system, each enforced rule has to be scrutinized whether it is essential. For instance, out of the ones you mentioned, does it really matter whether single or double quotes are used? Or whether opening bracket is on the same line or the next? A balance has to be sought after.

@DanielMSchmidt
Copy link
Contributor Author

I am sure you want good code too, sorry if it sounded otherwise. Prettier has no rules like eslint that a contributor needs to be aware of, it just reformats the code completely. You have some options there, but not too many.

It takes care of the code style that doesn't matter so much automatically, but it does not warn you about code smells. For things that matter to the quality of code we would need to have a tool like eslint. Prettier just tackled the conciseness

@LeoNatan
Copy link
Contributor

Ahh, this is interesting! Pardon the ignorance, I was not familiar. I thought it behaved like lint and broke the build. In that case, it actually sounds great.

One request, please make tabs the requirements, not spaces. 😄

@DanielMSchmidt
Copy link
Contributor Author

Sounds good, I would use the default otherwise, 👍

@LeoNatan
Copy link
Contributor

How will changes that prettier perform be committed and pushed? From CI?
It is also possible to set up client side git commit or push hooks (scripts). While those are “magic” of sorts, I think they might be easier to set up.

@DanielMSchmidt
Copy link
Contributor Author

You normally have a pre commit hook and a CI check so that the code base stays unpolluted

@LeoNatan
Copy link
Contributor

@DanielMSchmidt
Hey,
Was this implemented or abandoned? (I'm asking for the changelog generator)

@DanielMSchmidt
Copy link
Contributor Author

DanielMSchmidt commented Dec 19, 2017 via email

@LeoNatan
Copy link
Contributor

Great, thanks! 👍

@wix wix locked and limited conversation to collaborators Jul 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants