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 Git Hooks to run lint #1788

Merged
merged 4 commits into from
Aug 12, 2022
Merged

Conversation

PikachuEXE
Copy link
Collaborator

@PikachuEXE PikachuEXE commented Oct 4, 2021

Pull Request Type
Please select what type of pull request this is:

  • Feature Implementation

Related issue
Smaller version of #1171

Description
For new comers not coming from #1171
Git Hooks is Git has a way to fire off custom scripts when certain important actions occur. (quote from official doc)
There are client-side and server-side hooks but this PR is only about the client-side hooks.

For all available hook events and other stuff about Git Hooks see:
https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks

Summary of changes

  • Add git hook via lefthook
  • Git Hook scripts are generated by lefthook when installed via yarn, no extra step required
  • Add a pre-commit hook to run eslint on every commit which lints staged (about to be committed) files only (faster) where the filenames end with js/ts/vue

lefthook is picked as git hook manager due to

  • Written in Go
    • Go language makes Lefthook lightning-fast and provides support for concurrently executed scripts out of the box.
    • It can be installed without too many dependencies on many environments via different methods (But since we use yarn, just use yarn)
  • Run multiple commands in one hook, in sequence or in parallel (simple-git-hooks can't run commands in parallel)
  • Installing lefthook via npm/yarn will generate git hook scripts in your git hook path
    • No extra setup step = no chance for forgetting to setup git hook

Read the introduction post for lefthook if you interested in finding out more

PR Purpose
Lint task is sometimes NOT run by the PR submitter (including me!)
I know we have server side GH Action checks to ensure
So this is just an dev process improvement which should add just a little time to commit changes

Also #1171 is too big to review and merge

Screenshots (if appropriate)
Code Change Diff (for testing linting)
Screenshot 2021-10-04 at 3 27 02 PM

Git output
image

Testing (for code that is not small enough to be easily understandable)

  • Make change(s) violating any lint rule
  • Commit those changes

Desktop (please complete the following information):

  • OS: MacOS
  • OS Version: 11.6
  • FreeTube version: based on 84ac98f

Additional context
Add any other context about the problem here.

@PikachuEXE PikachuEXE marked this pull request as ready for review October 4, 2021 07:54
@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added the PR: waiting for review For PRs that are complete, tested, and ready for review label Oct 4, 2021
@PikachuEXE PikachuEXE changed the title Dev/githook Add Git Hooks to run lint Oct 4, 2021
@PrestonN PrestonN enabled auto-merge (squash) October 4, 2021 11:04
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels May 11, 2022
lefthook.yml Outdated Show resolved Hide resolved
lefthook.yml Outdated Show resolved Hide resolved
@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Jun 23, 2022
Co-authored-by: absidue <48293849+absidue@users.noreply.github.com>
Co-authored-by: absidue <48293849+absidue@users.noreply.github.com>
@PikachuEXE
Copy link
Collaborator Author

All requested changes in #1788 (review) made

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels Jun 24, 2022
@PikachuEXE
Copy link
Collaborator Author

@ChunkyProgrammer ~

@efb4f5ff-1298-471a-8973-3d47447115dc

@ChunkyProgrammer one more approval needed. Review if u have some spare time please

@ChunkyProgrammer
Copy link
Member

No idea how I missed the notifications for this one. This looks good to me. I'll approve it now. (There are newer versions of lefthook available so maybe it should be upgraded soon?)

@PrestonN PrestonN merged commit ba0bcee into FreeTubeApp:development Aug 12, 2022
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Aug 12, 2022
@PikachuEXE
Copy link
Collaborator Author

Crap I forgot to update the lefthook version

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

Successfully merging this pull request may close these issues.

5 participants