Together-100Devs / Together

Together is a group calendar application using the MERN stack intended to bring discord communities closer!
https://together.cyclic.app/
MIT License
167 stars 112 forks source link

Add lint-staged and husky to auto lint code before committing changes #317

Closed luciousmc closed 1 year ago

luciousmc commented 1 year ago

Please provide a summary of the feature

Currently, running the linter is a part of the steps to take before submitting a PR. The tools I'm suggesting will automate this so people won't have to think about doing it...

lint-staged: This will run eslint on files that have been staged for commit. If it runs into an error the contributor will not be able to commit the changes until they fix the errors.

husky: This will allow us to use git hooks. We will use the git hook, pre-commit, to run lint-staged whenever someone tries to commit changes.

Are there any potential issues you foresee with this feature

I discovered eslint was not picking up the prettier config so prettier has pretty much never been run on the code. If you run prettier you will see it flags a lot of errors. This can be fixed along with this issue, but since it will change pretty much every file, there are bound to be some conflicts that arise.

Additional context

Running eslint showed some things to fix image

After fixing the issue with eslint using prettier config this is the result image

The purpose here is to keep code syntax/formatting consistent throughout the codebase since we have a variety of people working on it.

Which Branch should this fix be Pulled into?

PR into development

If you're interested in working on this feature, please comment so you can be assigned This can be assigned to me.

Caleb-Cohen commented 1 year ago

Love this idea, and the team is on board with some form of implementation.

What if we set this to on push instead of on commit?

luciousmc commented 1 year ago

Once you commit changes they are no longer staged so they won't be picked up by lint-staged. Unless you mean to lint the whole codebase before pushing?

Caleb-Cohen commented 1 year ago

Once you commit changes they are no longer staged so they won't be picked up by lint-staged. Unless you mean to lint the whole codebase before pushing?

I guess? Assuming the code base is properly linted prior to the commits wouldn't the push effectively only lint all the staged commits?

luciousmc commented 1 year ago

No because lint-staged only targets staged(git add) files. Once you commit the files the stage is empty per se so it won't check anything. When you run eslint . it will check all of the files even if you only make a change to one.

Caleb-Cohen commented 1 year ago

No because lint-staged only targets staged(git add) files. Once you commit the files the stage is empty per se so it won't check anything. When you run eslint . it will check all of the files even if you only make a change to one.

Gotcha. Others have mentioned that some form of checking is possible on push instead of on commit. Are you familiar with how that would be possible?

luciousmc commented 1 year ago

Yea we can do that, but that would result in extra commits just for fixing lint errors. If thats no problemo then it's all good.

romanstetsyk commented 1 year ago

@luciousmc, are you working on this?

luciousmc commented 1 year ago

Yea, Caleb just assigned it to me the other day. I'll do it tonight.👍

romanstetsyk commented 1 year ago

Cool! I'm curious how you do this. I tried to add the lint-stage and husky to one of my projects and ran into some issues