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

Adjust CI scripts for package specific tasks #139

Merged
merged 18 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
name: lint-vizro-ai
name: checks-vizro-ai
defaults:
run:
working-directory: vizro-ai

on:
push:
branches: [main]
paths:
- "vizro-ai/**"
pull_request:
branches:
- "main"
paths:
- "vizro-ai/**"

concurrency:
group: lint-${{ github.head_ref }}
group: checks-${{ github.head_ref }}
cancel-in-progress: true

env:
Expand Down Expand Up @@ -42,9 +46,6 @@ jobs:
- name: List dependencies
run: hatch run all.py${{ matrix.python-version }}:pip freeze

- name: Lint
run: hatch run all.py${{ matrix.python-version }}:lint
maxschulz-COL marked this conversation as resolved.
Show resolved Hide resolved

- name: Check requirements for Snyk are up to date
run: |
pwd
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: lint-vizro-core
name: checks-vizro-core

defaults:
run:
Expand All @@ -7,12 +7,16 @@ defaults:
on:
push:
branches: [main]
paths:
- "vizro-core/**"
pull_request:
branches:
- "main"
paths:
- "vizro-core/**"

concurrency:
group: lint-${{ github.head_ref }}
group: checks-${{ github.head_ref }}
cancel-in-progress: true

env:
Expand Down Expand Up @@ -43,9 +47,6 @@ jobs:
- name: List dependencies
run: hatch run all.py${{ matrix.python-version }}:pip freeze

- name: Lint
run: hatch run all.py${{ matrix.python-version }}:lint

- name: Check schema is up to date
run: hatch run all.py${{ matrix.python-version }}:schema --check

Expand Down
47 changes: 47 additions & 0 deletions .github/workflows/lint-vizro-all.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
name: lint-vizro-all

defaults:
run:
working-directory: vizro-core # but could be any folder

on:
push:
branches: [main]
pull_request:
branches:
- "main"

concurrency:
group: lint-${{ github.head_ref }}
cancel-in-progress: true

env:
PYTHONUNBUFFERED: "1"
FORCE_COLOR: "1"

jobs:
run:
name: Python ${{ matrix.python-version }} on Linux
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ["3.8"]

steps:
- uses: actions/checkout@v4
with:
fetch-depth: ${{ github.event_name == 'pull_request' && 2 || 0 }}

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}

- name: Install Hatch
run: pip install --upgrade hatch

- name: List dependencies
run: hatch run all.py${{ matrix.python-version }}:pip freeze

- name: Lint
run: hatch run all.py${{ matrix.python-version }}:lint
Comment on lines +42 to +47
Copy link
Contributor

Choose a reason for hiding this comment

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

Since linting just requires pre-commit actually we should probably make a new deatched=True Hatch environment lint for it that doesn't include all of the dev dependencies. This way we could just do hatch run lint:lint on CI and it would be significantly faster. At the moment we install all the dependencies for all.py3.8 and then run pre-commit that uses only one of those dependencies which is a bit silly.

Not a big deal though, so feel free to do in a separate PR another time if you prefer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm good idea, let me see if I can incorporate that quickly still

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the other hand, hatch run lint is really used a lot, so would prefer not to have hatch run lint:lint. Or do you mean to keep pre-commit in the default environment as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's revisit in another PR if we still want to do changes

Copy link
Contributor

Choose a reason for hiding this comment

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

@maxschulz-COL yes, you're right - I originally put lint in the default environment precisely so that we don't have to write hatch run lint:lint. But probably the right way to do it is to make the hatch run lint:lint the "correct" command with a detached lint environment and then put a script "lint" = ["hatch run lint:lint"] in the default environment to just act as a shortcut so you can still do hatch run lint.

1 change: 1 addition & 0 deletions .github/workflows/test-integration-vizro-ai.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# run:
# working-directory: vizro-core
#
##### TODO: adjust below according to other scripts
#on:
# # push:
# # branches: [main]
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/test-integration-vizro-core.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ defaults:
on:
push:
branches: [main]
paths:
- "vizro-core/src/**"
maxschulz-COL marked this conversation as resolved.
Show resolved Hide resolved
pull_request:
branches:
- "main"
paths:
- "vizro-core/src/**"

concurrency:
group: test-integration-${{ github.head_ref }}
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/test-unit-vizro-ai.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ defaults:
on:
push:
branches: [main]
paths:
- "vizro-ai/src/**"
pull_request:
branches:
- "main"
paths:
- "vizro-ai/src/**"

concurrency:
group: test-unit-${{ github.head_ref }}
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/test-unit-vizro-core.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ defaults:
on:
push:
branches: [main]
paths:
- "vizro-core/src/**"
pull_request:
branches:
- "main"
paths:
- "vizro-core/src/**"

concurrency:
group: test-unit-${{ github.head_ref }}
Expand Down
Loading