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

Add documentation on how to use the identity hook #2180

Closed
DanKaplanSES opened this issue Jan 1, 2022 · 12 comments
Closed

Add documentation on how to use the identity hook #2180

DanKaplanSES opened this issue Jan 1, 2022 · 12 comments
Labels

Comments

@DanKaplanSES
Copy link

describe your issue

Based on the advice I received from #2179 I tried to add an identity hook to my script to troubleshoot why my configuration isn't working as expected.

This is the output of running pre-commit:

Firebase Identity....................................(no files to check)Skipped
- hook id: identity
Firebase Formatter...................................(no files to check)Skipped

I'm sure I'm not using this correctly, but there are no examples showing the right way to use this hook. I'd like the documentation to include an example.

pre-commit --version

pre-commit 2.16.0

.pre-commit-config.yaml

repos:
    - repo: meta
      hooks:
          - id: identity
            name: Firebase Identity
            language: system
            entry: bash -c 'cd firebase/functions && echo "$PWD"'
            pass_filenames: false
            types: [ts]
            files: ^firebase/functions/.*
    - repo: local
      hooks:
          - id: firebase-formatter
            name: Firebase Formatter
            language: system
            entry: bash -c 'cd firebase/functions && echo "$PWD"'
            pass_filenames: false
            types: [ts]
            files: ^firebase/functions/.*

~/.cache/pre-commit/pre-commit.log (if present)

This file didn't exist.

@asottile
Copy link
Member

asottile commented Jan 1, 2022

no files to check means that nothing has matched your filters so it's not even getting to the call because you've filtered out all the files

@DanKaplanSES
Copy link
Author

  1. Okay, but how do I troubleshoot this further? Should I re-create run -v should provide file search information for easy troubleshooting #2179 using the template?
  2. I appreciate your quick response, but what you think about adding an example to the documentation?

@asottile
Copy link
Member

asottile commented Jan 1, 2022

  1. wouldn't even help, it never gets there because there's no files
  2. you're using it correctly except that you've overridden entry -- you've just filtered to an empty set
  3. you haven't shown the command you're running, it's possible you're not using it correctly

@asottile
Copy link
Member

asottile commented Jan 1, 2022

my guesses:

  • maybe misunderstanding the command being run? pre-commit run -- by default runs on staged files, and you probably don't have any? -- perhaps you mean to use --all-files?
  • the file filtering: https://pre-commit.com/#filtering-files-with-types -- this has some advice on how to check what your files are identified as via identify-cli

@DanKaplanSES
Copy link
Author

DanKaplanSES commented Jan 1, 2022

I was running pre-commit directly.

by default runs on staged files

Ah, that's what i'm doing wrong. Once I modified a typescript file, it worked correctly. See, the documentation says:

4. (optional) Run against all the files

it's usually a good idea to run the hooks against all of the files when adding new hooks (usually pre-commit will only run on the changed files during git hooks)

        $ pre-commit run --all-files
        ...

But in my case, I need to execute the pre-commit script in a subdirectory. Therefore, the term --all-files, taken out of context, was the exact opposite of what I wanted to do. That makes me wonder something else: does --all-files literally run on all files, or just all matches?

wouldn't even help, it never gets there because there's no files

Perhaps you are correct, but I think it would help if verbose gave more information along the lines of #2179 If verbose told me somewhere it is looking for staged files, I would've instantly known what I was doing wrong. I think that would be more valuable than the documentation saying it, as the documentation is ~8500 words, and as far as I can tell, only a few of those words are used to explain this -- and they are wrapped in parentheses, so they are easy to pass by.

In my experience, verbose flags usually provide troubleshooting information, but for pre-commit -v run it only added the hook id and the hook run duration. Compare curl example.org to curl -v example.org as a counter example.

@asottile
Copy link
Member

asottile commented Jan 1, 2022

yeah, I guess I expected you to have gone through the quick start and understood the basics

@DanKaplanSES
Copy link
Author

yeah, I guess I expected you to have gone through the quick start and understood the basics

Well, actually I did. Not just this time, but a couple of months ago, too. Perhaps the first time I noticed the comment in the parentheses, and maybe this time I missed it. Or maybe I missed it both times, I can't remember.

Regardless, am I too take this as you're not interested in adding troubleshooting information to -v?

@asottile
Copy link
Member

asottile commented Jan 1, 2022

correct I do not want to expose the internals plus there are existing and established mechanisms for getting the same information

@DanKaplanSES
Copy link
Author

there are existing and established mechanisms for getting the same information

I'm still a little confused about that. I just changed my configuration file to look like this (notice how I removed the entry from the identity hook):

repos:
    - repo: meta
      hooks:
          - id: identity
            name: Firebase Identity
            language: system
            pass_filenames: false
            types: [ts]
            files: ^firebase/functions/.*
    - repo: local
      hooks:
          - id: firebase-formatter
            name: Firebase Formatter
            language: system
            entry: bash -c 'cd firebase/functions && echo "$PWD"'
            pass_filenames: false
            types: [ts]
            files: ^firebase/functions/.*

And when I ran the command line, this was its output:

➜  backend pre-commit run -v
Firebase Identity........................................................Passed
- hook id: identity
- duration: 0.03s
Firebase Formatter.......................................................Passed
- hook id: firebase-formatter
- duration: 0s

What information is identity supposed to provide?

Perhaps when you said, "there are existing and established mechanisms for getting the same information," you were referring to something that hasn't been brought up yet?

@DanKaplanSES
Copy link
Author

Oh, it might help to look at my git status:

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
	new file:   .pre-commit-config.yaml
	modified:   firebase/functions/src/types/<redacted>.ts

@asottile
Copy link
Member

asottile commented Jan 1, 2022

that would show you that there were no arguments passed to your hook -- if you remove pass_filenames you can see what it would pass

I am a little confused about your output -- it seems to have parts missing -- I expect:

$ pre-commit  run firebase-formatter -v
Firebase Identity........................................................Passed
- hook id: identity
- duration: 0.03s
Firebase Formatter.......................................................Passed
- hook id: firebase-formatter
- duration: 0s

/path/to/your/repo/firebase/functions

@DanKaplanSES
Copy link
Author

DanKaplanSES commented Jan 1, 2022

Ah, that makes sense.

I am a little confused about your output -- it seems to have parts missing -- I expect:

Oops, my bad. Something like that is on the last line.

Thanks for all the help with this. I understand your reasoning for not wanting to add more information to -v, but what would you think about an example of using the identity hook correctly? I could submit a PR for it if you want.

@pre-commit pre-commit deleted a comment from motin Feb 27, 2023
@pre-commit pre-commit locked as off-topic and limited conversation to collaborators Feb 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

2 participants