pyOpenSci / pyos-package-template

A Python package template that supports the pyOpenSci pure Python packaging tutorial
BSD 3-Clause "New" or "Revised" License
15 stars 11 forks source link

add initial git pre commit hook options #19

Closed xmnlab closed 4 years ago

xmnlab commented 4 years ago

Resolve #15

This PR adds some pre commit hooks options:

pre-commit-hooks are very useful to check code before commit.

let me know if these options are useful to pyopensci :)

choldgraf commented 4 years ago

I think this is cool! However I think we should definitely document all of this before exposing configuration options to folks. Could we add documentation for these to the dev guide so people know what they mean?

xmnlab commented 4 years ago

thanks for the feedback @choldgraf ! that makes a lot of sense. I will work on that!

I also was thinking that maybe would be good to test the pre-commit hook itself. to avoid to add a recipe if some bug. but I didn't find yet a good approach. any thought?

xmnlab commented 4 years ago

@choldgraf I opened a PR to add some information about git pre-commit hook into dev_guide: https://github.com/pyOpenSci/dev_guide/pull/57

choldgraf commented 4 years ago

re: tests, I think we could add a test that includes running pre-commit and checks whether there is a diff. I've used something like this in other projects (with github-actions):

  pre-commit:

    runs-on: ubuntu-latest

    steps:
    - uses: actions/checkout@v2
    - name: Set up Python 3.7
      uses: actions/setup-python@v1
      with:
        python-version: 3.7
    - name: Install dependencies
      run: |
        python -m pip install --upgrade pip
        pip install -e .[code_style]
    - name: Run pre-commit
      run: |
        pre-commit run --all-files || ( git status --short ; git diff ; exit 1 )
xmnlab commented 4 years ago

@choldgraf sounds good! I will work on that! thanks!

xmnlab commented 4 years ago

@choldgraf I added the test for github actions, but as I am not a collaborator for this repo, it was not triggered.

should I move it to travis-ci?

xmnlab commented 4 years ago

it seems that the github action was triggered .. but not listed here

choldgraf commented 4 years ago

Huh - that's weird, I'd have thought you could do this since you're a member of the POS organization. I just added you as a maintainer to this repo, can you try again?

xmnlab commented 4 years ago

thanks @choldgraf ! I cannot see the new actions here or in the previous list I receive the error: https://github.com/OpenScienceLabs/cookiecutter-pyopensci/actions

not sure what I am missing here .. should I create a workflow first direct in the actions tab?

choldgraf commented 4 years ago

hmmm, maybe it needs to be merged first before it'll work. Is this ready to merge otherwise? Then we can spot-check github actions in another PR?

xmnlab commented 4 years ago

I tested it locally ... so I think it is ready to merge! thanks! I will check in another PR the github actions.

choldgraf commented 4 years ago

sounds good! thanks for making this contribution @xmnlab :-)

xmnlab commented 4 years ago

thank you @choldgraf for all the recommendations and tips!

xmnlab commented 4 years ago

it seems the github actions worked: https://github.com/pyOpenSci/cookiecutter-pyopensci/runs/696767222?check_suite_focus=true