swellaby / rusty-hook Goto Github PK

Apparently the default toolchain on the Windows runners is now 32 bit msvc? 🤔

Will need to update the Windows PR job to account for this and get more explicit about which triple to use

Seems that even if there's compilation and/or test failures that the Windows PR job will still show as green anyway 🤦‍♂️

Given the crucial role these shell scripts play in the overall functionality, it would be great to include some automated tests that can be used to verify their behavior in our build and release pipelines.

Description

Skip the on-build hook setup when a CI environment is detected.

Value

We're currently checking to see if we're in a CI environment on hook execution, as we typically don't want to enforce git hooks in that context. This same notion applies to setup too: there's no need to create hooks and do the cli install of rusty-hook during the build/test process for applications using rusty-hook as a dev dependency

Description

Would be nice to print both the hook file version and perhaps the rusty-hook cli version on hook execution, in a follow up we can add an option to allow this to be disabled via the verbose config setting

Value

A user can see their current rusty-hook version via cli command (i.e. rusty-hook -v) but there's no way to see the version of the hook files outside of manually looking inside the underlying file within the .hooks directory

Up to this point cargo kcov has served us fairly well, but it's been causing some problems of late that aren't terribly surprising considerking that kcov's primarily focused on C/C++ code. We've notably had incorrect coverage reports on things like functions w/implicit expression returns and it's convinced me it is time to switch.

I've already done some local and CI testing with tarpaulin and confirmed it's working well (including the cases kcov was having challenges with) and believe that's the way to go.

Description

Currently, when the config file is missing, rusty-hook exits with an error code resulting in all the git hooks being blocked/rejected (which prevents a user from being able to commit, or requires them to include the -n or --no-verify flags).

We could update the generated hook script files to handle the "no config file" error scenario differently than other errors so that the git hooks are not rejected.

Value

This would allow for folks to partially remove rusty-hook from their project (only remove config file and dev-dep), but leave the rusty-hook scripts in their hooks directory in such a way that would not cause the git hooks to continue to be failed/rejected.

Still a few things to think through:

1. Should this be the standard/default script behavior, or an init option to let users decide for their own projects?
2. On partial removal, when a config file is missing, should the hooks continue silently or display a warning/message to the user before continuing?

IMO it's not ideal to keep a set of git hook scripts that are triggered on every git hook, running rusty-hook, checking for a config file, and then failing silently. I'd vote that we make this the new default behavior for the "no config file" scenario, and display a warning to the user on the pre-commit hook (not every hook) so that they know they should remove the script files too.

CC @traviskosarek @beverts312

We need to update the documentation to cover a few points:

1. Clearly convey that rusty-hook creates hook scripts in the .git/hooks directory
2. Enumerate the required steps to remove rusty-hook

Currently it can be confusing/painful for folks that want to remove rusty-hook from a project, especially if they were using the auto-install mode which makes rusty-hook a bit stickier than folks would expect. This can lead to a rather annoying and frustrating experience.

There's been some changes in the AZP ecosystem (Rust will be pre-installed on the hosted VMs, version 1.x of the pipeline upload/download tasks released, etc.) that should be applied to the pipeline templates

Latest clippy version is (thankfully) flagging some Option usages where we'd be better off using if let Some(...)...

The git hook script is currently defined inline in a Rust file, but it would be a bit easier to view/manage/etc. if we extract it into a separate .sh file and use include_str!(...) to pull the contents in at compile time. Plus this would also let us run tools like ShellCheck against the scripts

https://doc.rust-lang.org/std/macro.include_str.html

Description

If rusty-hooks is removed, it will corrupt your ./.git/hooks because there is no longer a .toml file w/hooks. This results in a cryptic “Failed to locate or parse config file” message after removal

Value

The output of rusty-hook should not rely on the existence of artifacts generated by rusty-hook, if rusty-hook is removed from a project, it should not screw up the project.

There's an updated version of the ci_info dependency we use

Description

Update the git hook script to auto install rusty-hook if it's not already available, allow this behavior to be disabled via an environment variable.

Value

#28 added support to automatically configure the git hooks on build. However, since cargo does not (yet) support installing dev deps as executables, there's still a need to run a one time cargo install rusty-hook manually. Adding this would truly make rusty-hook fully installable via simple dependency reference 👍

The Windows dev_setup script will currently do a force install of the VS vctools in order to support Rust's default MSVC target on Windows, if Rustc is not available.

The vctools install may not always be necessary, so if the script detects Rust has not been installed, it should check the minimum VS2013+ vctools install before attempting to install it.

The vs tools install almost always require a system restart, which means that in the event that the script does need to install the vctools, it cannot continue to the next steps prior to the user restarting.

This setup should be split out into a separate script/sequence to improve the dev experience

Description

Update the the git hook scripts to support upgrading the rusty-hook CLI conditionally, such as when the CLI version is less than X.

High level flow would be something like:

if cliVersion < minimumCliVersion; then 
  cargo install --force rusty-hook
fi

Value

The hook script files will be automatically upgraded, but the CLI is only installed if it doesn't already exist on the system. This could eventually lead to a lot of version drift between the script files and the CLI, and having the ability the conditionally upgrade the CLI version from the scripts would better enable keeping them more closely synchronized and simplifying the development/deployment process.

A new --output-dir option has been added to tarpaulin, so let's incorporate that into the pipeline that is generating code coverage

We've a new version of toml that's now being used so we should publish a new version with the updated dependencies

Description

Provide the ability for rusty-hook to automatically configure git hooks, perhaps via a build script

Value

Provides an easy option to have hooks automatically added on the first cargo fetch or test command

Environment Details

• Operating System
  • Linux
  • Mac
  • Windows
• Rust Version: n/a
• Rust Release Channel
  • Stable
  • Nightly
  • Beta
• Cargo Version: n/a
• rusty-hook Version: n/a

Description

The dev setup script on Windows is not working well, even after the fix attempt in #20/#26.

The detection of the VC build tools is not reliable, and there's issues with certain steps when the script is running behind a proxy

Description

Add shellcheck scan to our PR validation process (at least on either the Linux or Mac builds)

Value

We've got some shell scripts used as part of rusty-hook, both for the app and for dev support scripts. it'd be good to run some valdiation on those

We should include scripts as part of general dev setup to ensure LLDB is available for debugging.

For Linux:
https://lldb.llvm.org/resources/download.html

For Mac:
I believe it's installed by default with XCode, but also available via homebrew https://formulae.brew.sh/formula/llvm

For Windows:
https://chocolatey.org/packages/llvm

Environment Details

• Operating System
  • Linux
  • Mac
  • Windows
• Rust Version: n/a
• Rust Release Channel
  • Stable
  • Nightly
  • Beta
• Cargo Version: n/a
• rusty-hook Version: n/a

Description

The executable permission is not being set on the files created in the .hooks directory on unix systems

Description

Add executable permission to shell scripts in the repo

Value

Allows for the scripts to be run after cloning without having to manually chmod +x

Thanks for the suggestion @beverts312 😉

Description

Currently, every single git hook runs the setup checks (does the cli exists, does it meet the minimum required version, etc.). We should investigate optimizing those checks to only run on the "initiating" hook that starts the respective git hook lifecycles

Value

Running those checks on every single hook is inherently duplicative and unnecessary. For example, if the pre-commit hook check validates those items, then there's really no need to re-run the valdiation checks on the post-commit hook; you can't hit the post-commit hook without first hitting the pre-commit hook.

Add subcommand for removing hook files

Description

I run cargo make ci-flow as part of git pre push.
that script runs many tasks and takes a bit to finish.
it would be nice to see where it is in the execution, so if you would stream the githook script output in realtime instead of buffering it into memory and flushing only at the end, it would be really help and will improve the usability.

Value

Users for long running hooks won't think it is stuck and would get real time progress.

by the way, this crate is really great.

Self explanatory in the title 😄

I'm leaning towards just for this but still considering cargo-make

I'm still a big fan of Renovate, and even though they support Rust/Cargo, we should set up Dependabot for rusty-hook.

The biggest reason is that in addition to the core dependency management, Dependabot will also create PRs for dependencies with known security issues flagged in RustSec

This isn't obvious from the current docs, but users of rusty-hook have two ways of leveraging it in their projects.

1. more automatic/forced way - add it as a dev dependency, and then any dev/contributor on your repo will get the hooks set up automagically
2. more optional/opt-in way - instead of adding it as a dev dep, users could suggest/encourage devs/contributors to manually install (cargo install rusty-hook) and then run the rusty-hook init command if the dev/contributor wanted to set up local git hooks in his or her environment.

We certainly prefer to use option 1 in our personal repos, but given that rusty-hook gives flexibility/choice to the consumers, it's important for the docs to clearly convey those options so folks can choose the option that works best for them and their projects

Description

Allow for the hook configuration for a project to be augmented/overriden by a local/user configuration

Value

This would provide greater flexibility for individuals when working on a project. A project-level default could be set (using the existing .rusty-hook.toml configuration file), but then users could individually have their own configuration file (perhaps .rusty-hook.local.toml or something like that) if they wanted to run additional/different/etc. hooks in their local environment.

Publish a new patch version with the update dep versions on toml and ci_info

Description

rusty-hook should take the git params and expose them as an environment variable

Value

This would make those git params accessible to the downstream tools/scripts that are invoked by rusty-hook

Description

Add setup scripts, doc updates, etc. for code coverage

Value

Code coverage is an important part of the inner dev-loop process, and the PR validation checks enforce code coverage thresholds.

Accordingly, a developer working on this project needs to be able to generate code coverage info in his/her local env, and we should provide the relevant artifacts for them to be able to do so.

Let's publish a minor-bump version that includes the ci_info bump (#95) and the hook streaming output enhancement (#96)

getops dependency was bumped, need to pacakge publish a new 0.9.4 version