search

austintackaberry / ydkjs-exercises

Exercises to go along with You Don't Know JavaScript
MIT License
253 stars 74 forks source link

Add prettier git hook to ensure consistent styling #58

Closed austintackaberry closed 6 years ago

austintackaberry commented 6 years ago

Contributors might have different local styling configurations which is fine, but it can cause unnecessary changes that clutter up the diff.

If there was a git hook that applied a standard styling convention after committing changes, then the commits would be clean and would not include unnecessary non-standard styling changes.

nikrb commented 6 years ago

I can look into this. Thinking we should run test and lint before push.

austintackaberry commented 6 years ago

Yeah, that's a good idea. Maybe the test and lint should be opt-in? I don't want it to discourage first-time contributors from contributing

austintackaberry commented 6 years ago

https://github.com/austintackaberry/ydkjs-exercises/pull/64

austintackaberry commented 6 years ago

I noticed in recent commits that there were some styling changes that were made which suggests that perhaps the prettier styling isn't getting applied to the commits?

nikrb commented 6 years ago

if I recall correctly, I didn't apply the prettier changes initially as I expected there to be some changes regarding the styling. I think the pr was merged before I ran all the code through it, so I then thought the changes would come in piecemeal as the different files were edited.

If you feel it would be better, I could create a pr just to run prettier through all the files.

Also I noticed you changed some single quotes to double quotes. I think generally single quotes are used although I personally prefer double quotes, ala crockford :)

austintackaberry commented 6 years ago

Everyone has their own local prettier config which will change minor details like single to double quotes, spacing, etc. Personally, I am not too picky about styling. I just don't want to clutter up the diffs.

That is why we should have prettier run as a git hook

That way, regardless of how everyone's env is set up locally, the code on github will always be styled the same.

nikrb commented 6 years ago

I'm no expert with prettier, but the config is in the repo. Surely if everyone has their own local prettier config it's going to be chaos? It is run as a precommit git hook to the best of my knowledge, which is why files won't be pretty-fied until they are changed . I was suggesting we run all files through prettier as a pr as a starting point for future dev.

austintackaberry commented 6 years ago

Ah I see. So it is there, but it won't go into effect until each of the files are changed. My bad, I misunderstood. I'll re-close this issue.