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.

Sign up for GitHub

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

Jump to bottom

[SYNPY-1274] pre commit in gh action #1001

Merged
merged 8 commits into from
Oct 31, 2023
Merged

[SYNPY-1274] pre commit in gh action #1001

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a924eed
run pre-commit in test stage
BryanFauble Oct 31, 2023
bf52c24
run pre-commit action prior to test
BryanFauble Oct 31, 2023
9daa990
require pre-commit to pass before test
BryanFauble Oct 31, 2023
2991892
Introduce a change that should fail pre-commit
BryanFauble Oct 31, 2023
039cc1e
Revert "Introduce a change that should fail pre-commit"
BryanFauble Oct 31, 2023
97739b0
Pin the version of python being used
BryanFauble Oct 31, 2023
eb17f05
Pin to 3.9
BryanFauble Oct 31, 2023
3228991
Later version of actions
BryanFauble Oct 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 13 additions & 13 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,19 @@ on:

jobs:

pre-commit:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
BryanFauble marked this conversation as resolved.
Show resolved Hide resolved
- uses: actions/setup-python@v3
with:
python-version: 3.9
BryanFauble marked this conversation as resolved.
Show resolved Hide resolved
- uses: pre-commit/action@v3.0.0


# run unit (and integration tests if account secrets available) on our build matrix
test:
needs: [pre-commit]

strategy:
matrix:
Expand Down Expand Up @@ -65,7 +76,7 @@ jobs:
${{ steps.get-dependencies.outputs.site_bin_dir }}
key: ${{ runner.os }}-${{ matrix.python }}-build-${{ env.cache-name }}-${{ hashFiles('setup.py') }}-v7

- name: install-py-dependencies
- name: Install py-dependencies
if: steps.cache-dependencies.outputs.cache-hit != 'true'
shell: bash
run: |
Expand All @@ -80,11 +91,6 @@ jobs:
pip install numpy
fi

# - name: lint
# shell: bash
# run: |
# flake8

- name: run-unit-tests
shell: bash
run: |
Expand Down Expand Up @@ -138,15 +144,9 @@ jobs:
pytest -sv tests/integration/synapseclient/test_command_line_client.py
fi

# enforce the code matches the Black code style
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: psf/black@stable
# on a GitHub release, build the pip package and upload it as a GitHub release asset
package:
needs: [test,lint]
needs: [test,pre-commit]

runs-on: ubuntu-20.04

Expand Down
3 changes: 2 additions & 1 deletion .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
exclude: '^docs/conf.py'
exclude: '^docs/conf.py|^docs/build/'


repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
Expand Down
1 change: 0 additions & 1 deletion docs/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
sphinx
sphinx-argparse
sphinx_rtd_theme
-e .
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having this included was giving me:

ERROR: file:///home/bfauble/BryansGreatWorkspace/synapsePythonClient/docs (from -r requirements.txt (line 4)) does not appear to be a Python project: neither 'setup.py' nor 'pyproject.toml' found.

Also - the pre-commit wanted to move this to line 1 - which is what prompted me to look at this.

Copy link
Member

Choose a reason for hiding this comment

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

I think this was included only for a test read the docs integration.

Let's revisit the doc deployment strategy later on. It's an inconvenience that we have to build it ourselves when deploying - and our build doesn't allow for deployment of docs of past versions on our site.