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

Option to skip/control which hook files are created during init #119

Closed
calebcartwright opened this issue Sep 25, 2020 · 6 comments · Fixed by #147
Closed

Option to skip/control which hook files are created during init #119

calebcartwright opened this issue Sep 25, 2020 · 6 comments · Fixed by #147
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Pull Requests/Assistance welcomed!

Comments

@calebcartwright
Copy link
Member

Description

Add a feature that allows for controlling which hook files are instantiated

Value

Currently, the repo initialization process (triggered with either rusty-hook init or using compiling with rusty-hook as a dev dep) adds a hook file for all the supportable client side hooks to easily support user configurability. However, there are probably some advanced user scenarios where they'd like to control which hook files are/aren't created.

For example, there are other Rust tools that support creating their own hook files (for example git-journal which supports commit message hooks) and we should support users that may want to use rusty-hook in conjunction with those others. There's also the potential for some users to want really fine grained control (create the pre-commit file but none of the others)

Implementation Thoughts

I think the most common usage of this feature would be a skipping of one or two hook files (like commit-msg and perhaps prepare-commit-msg) so my initial inclination would be to support this via a new config option skip list, for example:

hook_file_skip_list = [
  "commit-msg"
]

If there's sufficient demand down the road to support an opt-in type feature over an opt out, we could add an additional config at that time for an include/allow list and figure out precedence if and when we get there (implementing this as a skip/opt-out won't preclude doing more work in the future).

Currently, the init process that sets up the hook files in the repo just iterates through the full list of hooks:

rusty-hook/src/hooks.rs

Lines 75 to 80 in 39fbca3

for hook in HOOK_NAMES.iter() {
let path = get_file_path(root_directory_path, hooks_directory, hook);
if write_file(&path, &hook_file_contents, true).is_err() {
return Err(String::from(HOOK_CREATION_ERROR));
};
}

(side note create_hook_files has a lot of duplication that could be consolidated into a separate closure or function)

That could be updated pretty easily to accept a list of hook files to skip, and then use the subset of HOOK_NAMES that weren't skipped (I suppose technically the disjunctive union provided the skip list doesn't have any junk entries)

Would then need to shuffle up the init order a bit to run the config creation check first, and then check for the hook_file_skip_list presence/value to pass along to setup_hooks function flow

pub fn init_directory<F, G, H>(
run_command: F,
write_file: G,
file_exists: H,
target_directory: Option<&str>,
) -> Result<(), String>
where
F: Fn(&str, Option<&str>, bool) -> Result<Option<String>, Option<String>>,
G: Fn(&str, &str, bool) -> Result<(), String>,
H: Fn(&str) -> Result<bool, ()>,
{
let root_directory_path = match git::get_root_directory_path(&run_command, target_directory) {
Ok(Some(path)) => path,
_ => return Err(String::from("Failure determining git repo root directory")),
};
if git::setup_hooks(&run_command, &write_file, &root_directory_path).is_err() {
return Err(String::from("Unable to create git hooks"));
};
if config::create_default_config_file(&write_file, &file_exists, &root_directory_path).is_err()
{
return Err(String::from("Unable to create config file"));
}
Ok(())
}

@calebcartwright calebcartwright added enhancement New feature or request help wanted Pull Requests/Assistance welcomed! good first issue Good for newcomers hacktoberfest Same as `good first issue` labels Sep 25, 2020
@calebcartwright calebcartwright removed the hacktoberfest Same as `good first issue` label Dec 18, 2020
@wpater
Copy link

wpater commented Nov 5, 2021

Is there any possibility to create a list of hooks to create? I would like to use only pre-commit and commit-msg but for now rusty-hooks creates all hooks, what is causing confusion.

@calebcartwright
Copy link
Member Author

what is causing confusion.

Sorry, I don't understand what you mean. Could you rephrase this question?

Is there any possibility to create a list of hooks to create?

It would be possible with this feature although it hasn't been released. Even if it were released, you'd have to use the skip option (or manually created config file) to enumerate all the other hooks that you didn't want; it's more of an opt-out than an opt-in.

rusty-hook's primary goal is to make git hooks easy so the users don't have to do or think about anything other than adding the commands to a config file, without having to run extra commands (or having to ask all your contributors to run commands) nor having to deal with the underlying git hooks yourselves. In order to accomplish this, yes, rust-hook establishes hook files, but it doesn't actually do any work unless you configure it as such.

For example, if you only want to run something on pre-commit and commit-msg, then you should only add those entries to your config file, just like the example in our readme; that example config file will result in hook commands only being run when the pre-commit, pre-push, and post-push git hooks are invoked, all other hooks are no-ops.

@wpater
Copy link

wpater commented Nov 8, 2021

Yes, indeed. My problem is that I have created this file first with configuration for only two hooks but rusty-hook created all possible hooks in .git/hooks - and in my opinion it's not right.

In default configuration (without file - file created by rusty-hooks) it's ok, we don't have information which hooks should be created and that's why rusty-hook creates all of them. But in case when file exists, and I explicitly configured that I want only 2 hooks - pre-commit and commit-msg - rusty-hook should create only those two hooks.

Did I miss something in configuration, or it is not possible? Do you plan to release a new version with this skip option?

@calebcartwright
Copy link
Member Author

Thank you for responding @wpater, however, I have to say that I don't feel like you really addressed the question. You'd already indicated that you don't want the hook files to be created, but you haven't explained why that's the case beyond sharing your
subjective opinion that it shouldn't.

There's a couple different styles of client side git hook tooling, and rusty-hook is absolutely in the style where creating all hook files is common and expected. This is done so that a single user can add a new hook command simply by checking in an update to the config file, and having that cascade to all users automatically (i.e. if you decide you want to introduce a new project-wide pre-push hook).

This is not without tradeoffs (like most choices in tech), but is an intentional decision to achieve the desired goals, and is always going to be our default behavior.

Did I miss something in configuration, or it is not possible? Do you plan to release a new version with this skip option?

As I stated in my previous comment this feature hasn't been released. It's currently backed up behind other changes in source that would break backwards compatibility and thus the release is blocked.

We will likely release it at some point once some of those blocking issues have been resolved, but it's not going to happen any time soon. For now, if you feel that strongly that the hook files shouldn't exist, then I'd suggest you consider alternative strategies/tooling, such as including hook file scripts in your repository with your own automation to copy or symlink them, or take a look at some of the alternative libs/tools we enumerate in our own Readme

@wpater
Copy link

wpater commented Nov 13, 2021

Ok, I understand you, thank you for your help.

@calebcartwright
Copy link
Member Author

Absolutely! Thanks for giving rusty-hook a try and sharing your perspective. Will be sure to drop a note here whenever we manage to get the new feature deployed so feel free to subscribe to this issue if that's something you're interested in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Pull Requests/Assistance welcomed!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants