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

chore: add devenv option to not install git hooks #1015

Merged
merged 1 commit into from
May 14, 2024

Conversation

doug-q
Copy link
Collaborator

@doug-q doug-q commented May 9, 2024

Tested with and without a devenv.local.nix with

{
  hugr.installGitHooks = false;
}

@doug-q doug-q requested a review from aborgna-q May 9, 2024 05:59
@doug-q
Copy link
Collaborator Author

doug-q commented May 9, 2024

Please feel free to improve my justfile, I'm quite clueless here.

Copy link
Collaborator

@aborgna-q aborgna-q left a comment

Choose a reason for hiding this comment

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

I'm on the fence on whether we should run poetry install directly in the devenv config, instead of adding complexity to the justfile.

  • Is there any usecase where we'd run just install false manually? The parameter gets confusing there.
  • Is it easy to do poetry install conditionally on the devenv.nix config?

@doug-q
Copy link
Collaborator Author

doug-q commented May 13, 2024

To be clear we are talking about a flag git-hooks=false. I am fine with the shell running poetry install (although not a huge fan), but installing git hooks is quite obnoxious(in my opinion). If I were manually running just install I would like to be able to suppress installing git hooks.

  • Is there any usecase where we'd run just install false manually? The parameter gets confusing there.

Yes, I would do this every time. it's just install git-hooks=false to be clear.

  • Is it easy to do poetry install conditionally on the devenv.nix config?

Yes, I think this is the best option

To be clear, I can disable git hooks in my local repo so it's not the biggest deal.

@aborgna-q
Copy link
Collaborator

We could have a

install:
    poetry install

recipe for the non-hooks variant, but that's just renaming things without adding much functionality.

@doug-q doug-q force-pushed the chore/configurable-git-hooks branch from 240abf7 to 559bcf2 Compare May 14, 2024 13:13
@doug-q doug-q force-pushed the chore/configurable-git-hooks branch from 559bcf2 to 9ac53ef Compare May 14, 2024 13:16
@doug-q
Copy link
Collaborator Author

doug-q commented May 14, 2024

@aborgna-q

I think we agree on setupInShell(although as always, better names welcome).

I don't think you'll like the justfile changes, but I'm trying my luck.

Copy link
Collaborator

@aborgna-q aborgna-q left a comment

Choose a reason for hiding this comment

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

I prefer it to the parameter version. Let's go with this 👍

@doug-q doug-q added this pull request to the merge queue May 14, 2024
Merged via the queue into main with commit 10e3158 May 14, 2024
13 checks passed
@doug-q doug-q deleted the chore/configurable-git-hooks branch May 14, 2024 13:59
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.

2 participants