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

Improve before and after internal logic #537

Merged
merged 2 commits into from
Dec 16, 2024
Merged

Conversation

sandydoo
Copy link
Member

@sandydoo sandydoo commented Dec 15, 2024

Consolidates and improves upon the work in #533 and #536.

  • Removes before and after from the raw config, initially added in feat: ordering in hooks. #533. These are not valid pre-commit configuration options and should not be here.
  • Removes code to filter out before and after from the raw config.
    There's nothing to filter out if they're not in the config in the first place. Essentially, reverts bugfix: remove before/after options from config json file #536.
  • Adds a pretty-ish error message (most of the diff) if the hooks cannot be sorted. Instead of erroring out on an obscure missing result field, we now show:
           error: The hooks can't be sorted because of a cycle in the dependency graph:
    
           nixpkgs-fmt -> shellcheck -> typos
    
           which leads to a loop back to: shellcheck
    
         Try removing the conflicting hook ids from the `before` and `after` attributes of these hooks:
    
           {
             nixpkgs-fmt = {
               after = [ ];
               before = [ ];
             };
             shellcheck = {
               after = [ ];
               before = [
                 "nixpkgs-fmt"
                 "typos"
               ];
             };
             typos = {
               after = [ ];
               before = [
                 "shellcheck"
               ];
             };
           }
    
  • Sets id to the attr name of the hook, instead of the name. This corrects a mistake made in the module transition (Move to new module structure #397). Technically, a breaking change, but we haven't had feedback from the initial mistake.
  • Exposes id in the options.

Consolidates previous work from #533 and #536.

- Removes `before` and `after` from the `raw` config. These are not valid
  pre-commit configuration options and should not be here.
- Removes code to filter out `before` and `after` from the `raw` config.
  There's nothing to filter out if they're not in the config in the
  first place.
- Set `id` to the attr name of the hook, instead of the `name`. This
  corrects a mistake made in the module transition (#397). Technically,
  a breaking change, but we haven't had feedback from the initial
  mistake.
- Expose `id` in options.
If the hooks form a cycle, we now throw an error with the hooks in
question and show their `before` and `after` fields to help identify the
cycle.
@sandydoo sandydoo changed the title Improve Improve before and after logic Dec 15, 2024
@sandydoo sandydoo changed the title Improve before and after logic Improve before and after internal logic Dec 15, 2024
@domenkozar domenkozar merged commit 0bb4be5 into master Dec 16, 2024
8 checks passed
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