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

check-manifest as pre-commit-hook #100

Closed
tobiasraabe opened this issue May 9, 2019 · 3 comments · Fixed by #102
Closed

check-manifest as pre-commit-hook #100

tobiasraabe opened this issue May 9, 2019 · 3 comments · Fixed by #102

Comments

@tobiasraabe
Copy link
Contributor

Hi,

thanks for the package. MANIFEST.in was always a mystery for me, but your package makes it better.

Could you make your package available as a pre-commit hook? It is simple as adding a .pre-commit-hooks.yaml to your package. The content is detailed here: https://pre-commit.com/#new-hooks.

The advantage is that people can easily integrate your package in their workflow and check before each commit whether the MANIFEST.in is sufficiently specified.

Best

@mgedmin
Copy link
Owner

mgedmin commented May 9, 2019

I thought it was up to the users to add a .pre-commit-hooks.yaml to their repositories if they want to run any tools such as check-manifest on every commit? I don't see how adding one to my repository would help anyone. (Unless as an example people could copy and paste?)

@tobiasraabe
Copy link
Contributor Author

Users add a pre-commit-config.yaml to their repository to specify their pre-commits. It includes a hypothetical section like this:

-   repo: https://github.com/mgedmin/check-manifest
    rev: 0.39.dev0
    hooks:
    -   id: check-manifest

But, to use your repository as the source, your repo must include the .pre-commit-hooks.yaml.

@mgedmin
Copy link
Owner

mgedmin commented May 9, 2019

Ah! That's neat!

I'd be happy to accept a pull request adding such a file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants