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

Feature: Ability to use multiple extends #126

Merged
merged 8 commits into from
May 19, 2020

Conversation

Evilweed
Copy link
Contributor

@Evilweed Evilweed commented May 8, 2020

Hello,
I would like to add the ability to use multiple extends. This was requested as a feature at my company as we are using Monorepo with many different projects (that are not separate git submodules), and using just one YML to manage all of those projects makes it become some GOD huge and unreadable file.

With this PR in effect, you can define and use multiple extends like this:

extends:
  - landing/lefthook-landing.yml
  - backend/lefthook-backend.yml

@Arkweid 🙏

@Arkweid
Copy link
Collaborator

Arkweid commented May 12, 2020

Thx for PR @Evilweed !
I will check it one current week, and if all ok going to made new release.

@Arkweid
Copy link
Collaborator

Arkweid commented May 14, 2020

@Evilweed looks fine! Could you add any tests? :)
Is it breaks after update existed configs?

@Evilweed
Copy link
Contributor Author

@Arkweid

  1. Tests are added;
  2. It was also tested manually, and it is backward compatible - you can either add 1 extend or multiple in array :)

@Arkweid
Copy link
Collaborator

Arkweid commented May 19, 2020

@Arkweid

  1. Tests are added;
  2. It was also tested manually, and it is backward compatible - you can either add 1 extend or multiple in array :)

Gj! Merged.

@Arkweid Arkweid merged commit 77816df into evilmartians:master May 19, 2020
@molszanski
Copy link

Yay!

@D1no
Copy link

D1no commented Oct 7, 2023

Just to clarify: This is not an extend but an inherit (left join), correct? When we have one root lefthook.yml that extends both a node js server and client project, they will overwrite one another and not both execute? I.e. when the command "format" is used in both like so:

# client/lefthook.yml
pre-commit:
  commands:
    format: # <-----------
      glob: "*.{js}"
      run: npm ....
# server/lefthook.yml
pre-commit:
  commands:
    format: # <-----------
      glob: "*.{ts}"
      run: yarn ....
# lefthook.yml
extends:
  - client/lefthook.yml
  - server/lefthook.yml

What will happen in this case to format? Do I need to tell engineers to make sure that commands are globally unique in the mono repo? Would be great if this is a true extend also with the ability to scope the ymls globs once to folders.

Couldn't find anything in the documentation — just evaluating lefthook right now and stumbled over this PR. For now I wrote this in an evaluation project as a draft:

# Workspace Specific Git Hooks (using lefthook)
#
#   This file is registered in:
#   infrastructure/development/githooks.yml
#
# Warning about Adding Hooks
#
#   (1) Make sure to add root: "folder/to/this/workspace" to limit the glob
#       pattern to this workspace only. Do not only use "*.{js..".
#       Otherwise the hook applies and conflicts outside this workspace folder.
#   (2) Use unique command names to avoid overwriting other githooks from other
#       workspaces with the same name. I.e. "platform-browser-format" instead
#       of just "format", which would be also used by a node.js server folder.
#

pre-commit:
  commands:
    platform-borwser-format:
      root: "platform/browser/"
      glob: "*.{js,jsx,ts,tsx,scss,html,css,md,json}"
      run: npx prettier --write --ignore-unknown {staged_files}
      stage_fixed: true

@mrexox
Copy link
Member

mrexox commented Oct 7, 2023

@D1no , you are right, this is more like a merge respecting the fields from the extends. The latter extend will overwrite the previous extends in your example. It's up to user to control this behavior.

I guess that you'd expect your example to finally show something like this?

# result
pre-commit:
  commands:
    format: 
      - glob: "*.{js}"
        run: npm ....
      - glob: "*.{ts}"
        run: yarn ....

If you feel the need for that please create a separate feature request. And I'll add it to my radar if many people want it because it feels like a big change in the lefthook design.

By the way, if you want to test what you get after all extends use lefthook dump command – it will print the config file after applying all extends.

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.

None yet

6 participants