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

Generic scripts for linting and testing #6

Merged
merged 8 commits into from
Feb 16, 2018

Conversation

pboothe
Copy link
Contributor

@pboothe pboothe commented Feb 16, 2018

Test hooks for json, yml, travis.yml, python, go

The yml and json are both raw hooks, but the others are even fleshed out


This change is Reviewable

@stephen-soltesz
Copy link

It looks like there is a similar pattern in the two files:

  • check for file type, take type-specific action.

pre-commit may fail, pre-commit-msg just outputs txt.

To prevent duplication or divergence between the files, I'm imagining a common file that manages each type-specific action called in a separate script (e.g. .git/hooks/cmd-lint-travis). The difference at call time would be "capture exit code" (for pre-commit) or "capture output" (for pre-commit-msg).

:lgtm: -- since having this now is better and we can work towards a solution with more share logic.


Reviewed 2 of 4 files at r1.
Review status: 2 of 4 files reviewed at latest revision, 4 unresolved discussions.


prepare-commit-msg, line 51 at r1 (raw file):

  if git ls-files | grep -q '\.py$'; then
    if [[ -x ./python-prepare-commit-msg ]]; then
      ./python-prepare-commit-msg

What is the PWD at this point? Above the PWD appears to be implicitly the root of the current repo. Here it looks like the .git/hooks/ dir? Or, it's a special case to handle running from the git-hooks repo itself? Please add a comment to clarify.


prepare-commit-msg, line 56 at r1 (raw file):

    else
      echo "No python-prepare-commit-msg found, but one should exist." 1>&2
      false

What does false do in this context?


prepare-commit-msg, line 64 at r1 (raw file):

  if git ls-files | grep -q '\.go$'; then
    if [[ -x ./go-prepare-commit-msg ]]; then
      ./go-prepare-commit-msg

Yeah, I'm still confused about PWD here.


README.md, line 8 at r1 (raw file):

to test.  The following commands will get that done:
```bash
  ln -s ../../git-hooks/pre-commit .git/hooks/pre-commit

Can I run these commands multiple times safely? Will it replace existing hooks?

Please prefer long form option names when available, i.e. --symbolic --force


Comments from Reviewable

@pboothe pboothe merged commit cfa0a3c into m-lab:master Feb 16, 2018
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