Skip to content

Create pre-commit hook#202

Merged
eldipa merged 2 commits intobyexamples:masterfrom
jeertmans:master
Nov 1, 2021
Merged

Create pre-commit hook#202
eldipa merged 2 commits intobyexamples:masterfrom
jeertmans:master

Conversation

@jeertmans
Copy link
Contributor

Hello,

I recently forked your repo to add a pre-commit hook to it, and use that for another project (see here).

In short, one can use byexample in his .pre-commit-config.yaml file:

repos:
-   repo: https://github.com/jeertmans/byexample
    rev: 10.3.2
    hooks:
    -   id: byexample
        args: ["-l", "python"]

As I think it may interest many people, I propose this pull-request.

Feel free to ask me for any change, or tell me if you dislike my proposition.

@eldipa
Copy link
Collaborator

eldipa commented Oct 29, 2021

Nice!, I'm not familiar with pre-commit so I will read some docs this weekend but I like the idea.

@jeertmans
Copy link
Contributor Author

Just to mention that if you are interested in pre-commit, I suggest you to also create (or I can do it for your) a config file for this repository such that future commits can use the pre-commit hook to avoid common programming mistakes or allow for a more unified syntax (using black linting for example).

Then, you can also setup automatic github actions or use the free pre-commit.ci service that will automatically run your pre-commit hook on Pull Requests (or more).

If you want more information about how it works, or on how to integrate it to your project, feel free to ask :)

Copy link
Collaborator

@eldipa eldipa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for suggesting pre-commit. Indeed seems to be an useful idea to integrate it with byexample.

I added a few rephrasesing. I'm okay with merging it as is and making the changes myself later but I wanted to show them to you for comments. (or if you want to do the changes, I'm okay too).

Let me know what you think.

@@ -0,0 +1,6 @@
- id: byexample
name: run code snippets in docstrings to validate them
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could rephrase this as "Run code snippets to validate them". I'm not 100% sure of my own proposal but I think that the word "docstring" suggests that it will only work for Python's docstrings which it's only one option.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, you are correct. I am most familiar with Python’s docstrings and I did not think of the fact that not all languages put their docs into strings or use this term :)

@@ -0,0 +1,6 @@
- id: byexample
name: run code snippets in docstrings to validate them
description: avoids writing errorneous code snippets.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it is correct that a failing example may mean an error in the code snippet, it is only one side of the coin. A failing example may mean that the snippet is correct but the source code that you are documenting is not.

Perhaps a better description could be:

byexample a literate programming engine where you mix ordinary text and snippets of code in the same file and then you execute them as regression tests.

You can always be sure that the examples are correct and your documentation is up to date!

I couldn't find where this information is shown or used by pre-commit but I guess that a multiline description like above should be supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I did not find much information about this neither. I mostly got inspiration from pre-commit’s pre-commit-hooks.
However, I don’t think the description matters much as it won’t be read by many people: in my opinion, people that use per-commit hooks already know what the hooks are doing or should get more information elsewhere than in that very file.

@eldipa eldipa merged commit 35245c1 into byexamples:master Nov 1, 2021
eldipa added a commit that referenced this pull request Nov 1, 2021
@eldipa
Copy link
Collaborator

eldipa commented Nov 1, 2021

@jeertmans merged!

The feature is available from version 10.4.0, released a few moments ago (see https://github.com/byexamples/byexample/releases/tag/10.4.0 ) and the documentation of the feature is in in https://byexamples.github.io/byexample/recipes/pre-commit .

Thanks a lot for the contribution!

@jeertmans
Copy link
Contributor Author

You are welcome, and thanks for your involvement in my PR!

byexample is a great project and I am thankful it exists ;)

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