Adding configuration for pre-commit

[sscheib]

I don't know if you'd like to entertain the idea, but I thought I'll give it a try.

With this pull request I am adding a hooks configuration for pre-commit, which is a widely used tool for configuring git hooks.

I think it makes a lot of sense to run pyspelling before committing code, therefore I'd like to contribute it.

Please let me know your thoughts.

[facelessuser]

I think this makes sense for users or organizations that want such behavior. As things currently are, contributors of this project go through CI which tests spelling already. While I won't say I never commit directly to main 😏, I usually go through the same PR process. I currently find this sufficient. If I had multiple core maintainers, and I allowed them all to push directly to main, I can see where requiring a commit hook would make more sense. So as things currently stand, the git hook would probably just slow me down and not offer anything that I'm not already getting from CI.

I do think such a suggestion is helpful for the right projects where something like this needs to be more strongly enforced.

[sscheib]

I just wanted to add it to my very own .pre-commit-config.yaml so that I can specify a repository and not adding a local hook to it.
This makes using pyspelling more unified (at least for my pre-commit usage).

The pre-commit configuration is primarily for the users of pyspelling, not necessarily for the maintainers 🙂.

After all, pre-commit is voluntary. You cannot enforce that (to my knowledge at least). For that reason, changes go through a PR where workflows ensure that everything as it should be.

Please don't confuse the .pre-commit-hooks.yaml with .pre-commit-config.yaml. The latter is a configuration to run pre-commit before/after certain stages while using git.

The former, .pre-commit-hooks.yaml simply defines a way a hook can be used by including it in a .pre-commit-config.yaml

pre-commit is basically the 'first stage' before you actually commit (if you opt-in for using it).

I thought it doesn't hurt to add it, since it is basically no maintenance whatsoever, but eases the initial usage when you'd like to use it without setting up your own virtual environment (pre-commit does that on its own).

I initially planned to simply create a repository which provides the .pre-commit-hooks.yaml, but, unfortunately, pre-commit requires a pyproject.toml or a setup.py in the repository that provides information how to install the application of the hook.

Sure, I can copy the pyproject.toml to my repository, but I'd always lag behind, since the actual work happens in this repository with regards to pyspelling.

I hope I explained a little better what I had in mind when proposing this PR.
I'd be very happy if you'd consider merging it!

[facelessuser]

I'm not sure I understand. You've committed a pre commit yaml config to this project, but most people will install through pypi, so how will they get your config? I don't often use such functionality, so forgive me if I seem a little clueless on this topic.

[sscheib]

No worries at all.

Let me try to explain it a little better.

All pre-commit does is to hook (via git hooks) before/after certain git stages. Such a stage could be, for instance, "before a commit" (hence: pre-commit).

It creates a virtual environment to install the application to run before committing code. In this case that's pyspelling.

The file I introduce with this PR is the definition of how pre-commit has to install and run this application in its own virtual environment.

To utilize this, users have to create a .pre-commit-config.yaml in their own repository (here is an example of one of my repositories using it: https://github.com/sscheib/ansible-role-file_deployment/blob/main/.pre-commit-config.yaml) and reference the repository the hook definition can be found - which would be this repository here.

Should you merge this PR and build a new version, I'd update my .pre-commit-config.yaml like the following:

  - repo: 'https://github.com/facelessuser/pyspelling'
    rev: 'v2.11'
    hooks:
      - id: 'pyspelling'
        name: 'pyspelling'

This has the benefit that whenever you release a new version of pyspelling I don't have to update it manually myself (by running pip3 install --upgrade pyspelling). I can use something like renovate or pre-commit.ci, which automatically are creating PRs (if configured to do so) to update the revision (rev) in my .pre-commit-config.yaml of my repository.

Former, existing and future users and maintainers of pyspelling that do not use pre-commit are not confronted with it in any way. It is merely for those who'd like to use pyspelling in pre-commit for their own repositories. All that is provided by this file, again, is the definition of the hook 🙂.

I hope I did a better job this time of explaining it.
Please don't hesitate to come back to me if you have any concerns or doubts!

[facelessuser]

Your referencing the repo directly, not installing through PyPI? Usage outside of PyPI, well not prohibited, is not quite how this tool is designed and maintained to be used.

[sscheib]

It is ultimately installed through pip, via pre-commit (in a virtual environment). The repository reference is for the .pre-commit-hooks.yaml.

[facelessuser]

Right, but you have to reference the repo to get it, not the pip installed package it seems. Also, there seems to be nothing in the config that is release dependent, so why is it better for us to provide it instead of you defining it in your repo?

[sscheib]

The command that is run by pre-commit after checking out the repository, is the following (to the best of my knowledge):

pip3 install .

This way it retrieves the requirements/dependencies of pyspelling (via pyproject.toml). So, as said, ultimately, it is installed from pypi.org.

in the config that is release dependent, so why is it better for us to provide it instead of you defining it in your repo?

If I define it "locally" in my repository, I need to keep up with the changes of pyspelling. Once you issue a new release, I need to manually check for it and edit all my .pre-commit-config.yaml files manually in all my repositories.

Creating my own repository to provide merely this hook configuration has the same culprit, as I need to provide pre-commit the pyproject.toml in my repository so that it knows what to install. This also introduces manual effort.

If it's part of a release of pyspelling (simply as the file laying around in this repository), a bot such as pre-commit.ci can automatically update the revision to the latest for me in all my repositories. The latest version is determined through the repository tags.

Does that clear things up?

[facelessuser]

The PyPI package, from pip, does not include the hook. It will not be in the wheel. I don't quite understand how you are using the project, but I think you are doing something extra here. Again, there appears to be no release dependent info in the config either, so I also still don't understand why it needs to be included in PySpelling directly.

[sscheib]

The PyPI package, from pip, does not include the hook. It will not be in the wheel. I don't quite understand how you are using the project, but I think you are doing something extra here.

That's absolutely correct.

What pre-commit does is essentially:

• create a virtual environment
• checkout the repository specified in the hook in a specific version (e.g. v2.10)
• use the included pyproject.toml (or setup.py, whatever is present) to install the application from pypi.org with all required dependencies

I don't have the pyproject.toml in my own repository if I were to create my own repository to provide the hook.
I'd need to manually copy it and keep it updated. The benefit of having the hook definition in the repository that provides the application (in this case pyspelling) is exactly for that reason: You have the dependencies defined and up-to-date.

I can also list you a few projects that make use of it exactly like that:

• yamllint
• markdownlint
• markdown-link-check
• ansible-lint

There are many more; These are just the ones I use.

I think we are running in circles. If you'd rather not have the PR merged: Please let me know - all good! 🙂

I'd just like to see pyspelling added as an essential tool to use before committing quality code - pre-commit is a perfect solution for that. Especially since it does not interfere in any way with the current usage.

[facelessuser]

I can also list you a few projects that make use of it exactly like that:

This helps. Let me look into this more before I come back with a response.

[facelessuser]

Okay, I understand a bit better now. Forgive the circles we had to run around, but I have to understand what I am committing to maintain before I opt to do so. I think I'm probably okay with idea of this.

Unfortunately, there is still one rub, if we do this, does this mean you are pulling the tip of main all the time? If so, you do so at your own risk as I do not guarantee that you will not have a bug introduced on the tip. Or am I still misunderstanding?

If I am correct, then what you may be asking is for me to not only add the hook, but change my current maintenance habits, to develop off main only ( a separate dev branch). That is a bigger ask.

[sscheib]

Okay, I understand a bit better now. Forgive the circles we had to run around, but I have to understand what I am committing to maintain before I opt to do so. I think I'm probably okay with idea of this.

All good. I just had the feeling you were arguing against the idea - which is perfectly fine. I just wanted to avoid that we are running in circles, without getting to a common understanding.

Unfortunately, there is still one rub, if we do this, does this mean you are pulling the tip of main all the time? If so, you do so at your own risk as I do not guarantee that you will not have a bug introduced on the tip. Or am I still misunderstanding?

No, that's not the case, although you could configure pre-commit this way. This, however, is typically not how it is used like.

The pre-commit configuration has an attribute rev. This can be set to either:

1. A specific branch (which is never a good idea)
2. A specific release/tag (which is what most people do)
3. A specific commit digest (to be extra "safe" in case a bad actor replaces a release)

From my experience most people use option 2: Set rev to a specific release, e.g.:

- repo: 'https://github.com/facelessuser/pyspelling'
    rev: 'v2.11'  # this is a hypothetical new release and would refer to git tag v2.11
    hooks:
      - id: 'pyspelling'
        name: 'pyspelling'

You are already creating tags using git (git tag), which I (and others) can leverage for the hook perfectly.

Which brings me to the last quote:

If I am correct, then what you may be asking is for me to not only add the hook, but change my current maintenance habits, to develop off main only ( a separate dev branch). That is a bigger ask.

No, you don't need to change your developing habits at all!

Let me know if you have any questions or doubts.

[facelessuser]

Nope, this clears up everything. I think now that I have an understanding, I am less squeamish about the change. I have not traditionally taken advantage of git hooks in this way, so I really wanted to get a grasp of what I was getting into.

With this all sorted and clear, I think I'm okay with this change.

[facelessuser]

We would need to note this in the documentation though.

[sscheib]

Sure, I can do that. Where would you like to have that added?

[facelessuser]

Maybe in its own section at the bottom of this page: https://github.com/facelessuser/pyspelling/blob/master/docs/src/markdown/index.md.

Just an example showing how someone may use it.

[sscheib]

What about something like the following?

GitHub kind of messes up the markdown, but I think you get the idea 😅.

## Usage as pre-commit hook

`pyspelling` can be used as [`pre-commit`](https://pre-commit.com/) hook. To use it as `pre-commit` hook, please have a look at the following example `.pre-commit-config.yaml`:

```yaml
repos:
  - repo: 'https://github.com/facelessuser/pyspelling.git'
    rev: 'v2.11'
    hooks:
      - id: 'pyspelling'
        verbose: true
        pass_filenames: true
...

Please note that version tags should be preferred over using the `master` branch as revision (`rev`) attribute, as the `master` branch is considered unstable.

[sscheib]

If that's fine with you, I'd add this change to this PR. I am also happy to create a new PR for that change. Just let me know how you'd prefer it 🙂.

[facelessuser]

I'd do it here. It is all related. I'm fine with the proposed text. Maybe titlecase the title and that should be fine.

[sscheib]

I upper-cased 'hook' but not pre-commit since that's - as far as I know - the intended spelling.

[facelessuser]

Maybe place it in `pre-commit`?

[sscheib]

Sure, I hesitated to do so, as some people don't like code blocks in their headings and didn't see it anywhere else in the documentation (on a quick glimpse, however). I'll push an update momentarily.

[sscheib]

I just also saw that you are writing PySpelling rather than pyspelling - I corrected that also.

[facelessuser]

as some people don't like code blocks in their headings

I don't mind. Code stuff goes in code stuff 🙂. I know even when people do that, they sometimes won't style it as code in titles either, and I say 🤷🏻.

[sscheib]

Sorry for the mess, I just saw I messed up the configuration in the markdown. It is now correct - double checked it.

I don't mind. Code stuff goes in code stuff 🙂. I know even when people do that, they sometimes won't style it as code in titles either, and I say 🤷🏻.

I see it exactly the same way. Sometimes it looks a little weird, especially when it is only a title with an inline code block, but, hey, code goes into code blocks - no matter what 🤣

[facelessuser]

@gir-bot lgtm