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

Use prettier for code style guide #578

Closed
ChadBailey opened this issue Jan 12, 2020 · 4 comments
Closed

Use prettier for code style guide #578

ChadBailey opened this issue Jan 12, 2020 · 4 comments

Comments

@ChadBailey
Copy link
Collaborator

ChadBailey commented Jan 12, 2020

I think creating a full fledged style guide, or even expecting contributors to adhere to a common style guide is probably too much for a project of this size. That said, I do think the code base could benefit greatly from improved styling. Today there are a lot of minor formatting errors like mismatched indentation levels, etc. These have no impact on the functionality of the code, but it certainly makes contributing more difficult.

For these reasons I think some kind of easy to follow style guide should be chosen for the project. I think the best option may be using something like the prettier formatter. This would simplify styling while improving consistency and legibility.

What do you all think?

I have created a PR #579 to demonstrate how this would look

@igrigorik
Copy link
Owner

I like it. Enforcing a style guide would help eliminate a bunch of unnecessary back-n-forth on nitpick style comments, and consistency is great.

I would suggest we explore two options here:

  1. Setup a PR validation step that runs a check and blocks any PR if it's not formatted correctly
  2. Or, setup a bot to autoformat on commit

Between the two, I think my preference is (1) as it results in less surprises to folks committing code, even though it requires a bit more work from them.

@ChadBailey
Copy link
Collaborator Author

I agree, great thoughts. I wonder if there's a way to have it block the PR but give the option of having a bot autoformat it for them if they don't want to bother figuring it out for themselves.

I'll take a closer look at the options when I find the time.

@igrigorik
Copy link
Owner

Indeed, that would be ideal, but I wouldn't consider that a blocker. We could check-in a formatter config and provide some instructions on how to install and run it.

Thanks for digging into this!

@ChadBailey
Copy link
Collaborator Author

I've submitted a PR... unfortunately, it does not accomplish blocking an ill-formatted PR though. I couldn't figure out how to do that (I'd love to learn if you have some suggestions of where I can look). I have come up with what I'm hoping is an acceptable compromise of creating a pre-commit hook

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

2 participants