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

NEW Provide pre-commit hook #100

Merged
merged 3 commits into from
Feb 21, 2024
Merged

NEW Provide pre-commit hook #100

merged 3 commits into from
Feb 21, 2024

Conversation

mdeweerd
Copy link
Contributor

NEW Provide pre-commit hook

This adds a pre-commit hook definition so that this repository can be easily used through pre-commit

# NEW Provide pre-commit hook

This adds a pre-commit hook definition so that this
repository can be easily used through pre-commit
@pgrange
Copy link
Owner

pgrange commented Feb 20, 2024

Hi @mdeweerd,
Thank you for your PR.

How is it related to #97? Is it a replacement and should we just close #97?

Also, I'm sorry, I don't know what pre-commit, could explain shortly how you think this would add value to bash_unit?

Thank you so much for your help.
Cheers,

@mdeweerd
Copy link
Contributor Author

#97 is the core pre-commit setup that uses third-party hooks to execute actions when you commit.

#100 Proposes bash-unit as a hook that can be added to a pre-commit setup to run bash-unit upon the commit action (useable by other project).

#97 could be updated to add the use of the bash-unit hook but then #100 needs to be available first in a way. You could accept #100 and #97 and if you want I'll add the use of this project's own hook to its pre-commit setup.

- id: bash-unit
----

== Running `+bash_unit+` in the test script
Copy link
Owner

Choose a reason for hiding this comment

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

I would remove this section in the documentation as test_doc, in my opinion, is really advanced and odd use of bash_unit. One should not have to do that in 99.99% of the cases so that describing it here introduces some sort of noise in the doc, in my opinion and can even be a bit scary for new users :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

README.adoc Outdated
@@ -78,6 +78,50 @@ You can also download it from the https://github.com/pgrange/bash_unit/releases[

endif::[]

== pre-commit hook
Copy link
Owner

Choose a reason for hiding this comment

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

I would add an extra = so that this becomes a sub-section of the installation section. Is it fair to say that running it with pre-commit hook is an alternative way of installing it?

README.adoc Outdated Show resolved Hide resolved
@pgrange pgrange merged commit 7ab0816 into pgrange:master Feb 21, 2024
@mdeweerd mdeweerd deleted the provide-pre-commit-hook branch February 21, 2024 17:04
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