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

Fix typo in pre-commit config in README #1424

Merged
merged 1 commit into from
May 13, 2024

Conversation

f-hollow
Copy link
Contributor

First of all, thank you for this very useful tool!

I ran into an issue while trying to add lychee to the pre-commit hook. Here is the account of the steps that I took.

  1. I copied the following pre-commit config code from the README into my project's .pre-commit-config.yaml:
    # .pre-commit-config.yaml
    repos:
      - repo: https://github.com/lycheeverse/lychee.git
        rev: 0.15.1
        hooks:
          - id: lychee
            # Optionally include additional CLI arguments
            args: ["--no-progress", "--exclude", "file://"]
  2. While committing files, pre-commit returned this:
    An unexpected error has occurred: CalledProcessError: command: ('/usr/lib/git-core/git', 'checkout', '0.15.1')
    return code: 1
    stdout: (none)
    stderr:
        error: pathspec '0.15.1' did not match any file(s) known to git
    Check the log at ...
  3. Updating the line rev: 0.15.1 to rev: v0.15.1 fixed the issue.

Hence, this PR is offered to fix the typo in the provided pre-commit config.

@f-hollow f-hollow force-pushed the fix-typo-in-pre-commit-config branch from 43786b7 to bb6f08d Compare May 13, 2024 06:04
@f-hollow
Copy link
Contributor Author

Not sure why certain CI jobs failed. Please let me know if the issues are related to my changes.

@dscho
Copy link
Contributor

dscho commented May 13, 2024

I was as puzzled as you in my PR. Turns out that outside changes (a clippy update?) require fixes such as the ones I proposed in #1423.

@f-hollow
Copy link
Contributor Author

That's a bit too deep for me.

In this case, should this PR wait till you merge the PRs in question and then do a rebase? Or what would you suggest we do?

@dscho
Copy link
Contributor

dscho commented May 13, 2024

@f-hollow I am only a contributor, not a maintainer, so I don't know.

@mre
Copy link
Member

mre commented May 13, 2024

@dscho fixed it, so this branch can be rebased. (Thanks @dscho.)
I'm guessing it's because of the recent Rust 1.78 release, which activated some more clippy lints.

@mre
Copy link
Member

mre commented May 13, 2024

Oh, and for some context as to why this is broken, it's because of the changes in lycheeverse/lychee-action#204. We forgot to update the lychee docs. Thanks for fixing.

Checked the change again, and it's unrelated to the linked PR.
I think it's always been broken, actually? Either way, glad it's fixed.

Will merge, as rebase won't change much anyway, because we don't have a test for this.

@mre mre merged commit 9e031b6 into lycheeverse:master May 13, 2024
4 of 7 checks passed
@f-hollow
Copy link
Contributor Author

@dscho Oh, I assumed by default that your were a maintainer. Sorry!

@mre Thank you for merging my PR!

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.

None yet

3 participants