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

CI: Trigger workflows based on changed files #2952

Merged
merged 8 commits into from
Jan 6, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 2 additions & 6 deletions .github/workflows/benchmarks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,9 @@ on:
branches: [ main ]
paths:
Copy link
Member Author

Choose a reason for hiding this comment

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

We need to run benchmarks for changes of test files. The main reasons are: (1) when making changes to a test in a PR, we need to know that the new test is not too slow. (2) when merging the changes into the main branch, benchmarks need to be triggered to update the performance baseline. Otherwise, when benchmarks are triggered in a new PR, we will be surprised to see that the test has a performance change even though the changes in the PR is irrelevant to the test.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, #2945 was a good example of this where we only modified the test files, and that caused a change in performance. But this is a bit of a rare case though. Maybe we only need to run the benchmark on merge into main, but not in the Pull Request itself? I.e. delete L14 but keep L19?

Copy link
Member Author

@seisman seisman Jan 6, 2024

Choose a reason for hiding this comment

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

Maybe we only need to run the benchmark on merge into main, but not in the Pull Request itself?

Or just disable the workflow in PR (i.e. remove lines 16-20) and run it via slash command when necessary?

Copy link
Member

@weiji14 weiji14 Jan 6, 2024

Choose a reason for hiding this comment

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

Or just disable the workflow in PR (i.e. remove lines 16-20) and run it via slash command when necessary?

Yeah, let's go with that. Just keep a commented line like # pull_request:, so we can manually trigger the benchmark workflow if needed. The slash command can be done in a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in b1e5fa8.

- 'pygmt/**/*.py'
- '!pygmt/tests/**'
- '.github/workflows/benchmarks.yml'
pull_request:
paths:
- 'pygmt/**/*.py'
- '!pygmt/tests/**'
- '.github/workflows/benchmarks.yml'
# Uncomment the 'pull_request' line below to trigger the workflow in PR
# pull_request:
seisman marked this conversation as resolved.
Show resolved Hide resolved
# `workflow_dispatch` allows CodSpeed to trigger backtest
# performance analysis in order to generate initial data.
workflow_dispatch:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/cache_data.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
# by other GitHub Actions workflows.
#
# It is scheduled to run every Sunday at 12:00 (UTC). If new remote files are
# needed urgently, maintainers can update the workflow file or
# needed urgently, maintainers can update the workflow file or the
# 'pygmt/helpers/caching.py' file to refresh the cache.
#
name: Cache data

on:
pull_request:
# Make any changes to the following files to refresh the cache
# Make any changes to the following files to refresh the cache in PRs
paths:
- 'pygmt/helpers/caching.py'
- '.github/workflows/cache_data.yaml'
Expand Down
19 changes: 14 additions & 5 deletions .github/workflows/ci_docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,22 @@ name: Docs
on:
push:
branches: [ main ]
paths:
- 'pygmt/**/*.py'
- '!pygmt/tests/**'
- 'doc/**'
- 'examples/**'
- 'README.rst'
- '.github/workflows/ci_docs.yml'
weiji14 marked this conversation as resolved.
Show resolved Hide resolved
pull_request:
types: [opened, reopened, synchronize, ready_for_review]
paths-ignore:
- 'pygmt/tests/**'
- '*.md'
- 'LICENSE.txt'
- '.gitignore'
paths:
- 'pygmt/**/*.py'
- '!pygmt/tests/**'
- 'doc/**'
- 'examples/**'
- 'README.rst'
- '.github/workflows/ci_docs.yml'
weiji14 marked this conversation as resolved.
Show resolved Hide resolved
release:
types:
- published
Expand Down
13 changes: 6 additions & 7 deletions .github/workflows/ci_tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,14 @@ name: Tests
on:
push:
branches: [ main ]
paths:
- 'pygmt/**'
- '.github/workflows/ci_tests.yaml'
pull_request:
types: [opened, reopened, synchronize, ready_for_review]
paths-ignore:
- 'doc/**'
- 'examples/**'
- '*.md'
- 'README.rst'
- 'LICENSE.txt'
- '.gitignore'
paths:
- 'pygmt/**'
- '.github/workflows/ci_tests.yaml'
release:
types:
- published
Expand Down
10 changes: 3 additions & 7 deletions .github/workflows/ci_tests_dev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,9 @@ on:
# branches: [ main ]
pull_request:
types: [ready_for_review]
paths-ignore:
- 'doc/**'
- 'examples/**'
- '*.md'
- 'README.rst'
- 'LICENSE.txt'
- '.gitignore'
paths:
- 'pygmt/**'
- '.github/workflows/ci_tests_dev.yaml'
repository_dispatch:
types: [test-gmt-dev-command]
# Schedule tests on Monday/Wednesday/Friday
Expand Down
11 changes: 4 additions & 7 deletions .github/workflows/ci_tests_legacy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,12 @@ name: GMT Legacy Tests
on:
# push:
# branches: [ main ]
# Uncomment the 'pull_request' line below to trigger the workflow in PR
# pull_request:
seisman marked this conversation as resolved.
Show resolved Hide resolved
# types: [ready_for_review]
# paths-ignore:
# - 'doc/**'
# - 'examples/**'
# - '*.md'
# - 'README.rst'
# - 'LICENSE.txt'
# - '.gitignore'
# paths:
# - 'pygmt/**'
# - '.github/workflows/ci_tests_legacy.yaml'
# Schedule tests on Tuesday
schedule:
- cron: '0 0 * * 2'
Expand Down
11 changes: 9 additions & 2 deletions .github/workflows/publish-to-pypi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,15 @@ name: Publish to PyPI
# Only run for pushes to the main branch and releases.
on:
push:
branches:
- main
branches: [ main ]
Copy link
Member Author

Choose a reason for hiding this comment

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

Only publish a test release to TestPyPI if there are code changes.

paths:
- 'pygmt/**'
seisman marked this conversation as resolved.
Show resolved Hide resolved
- '!pygmt/tests/**'
- 'Makefile'
- 'MANIFEST.in'
- 'pyproject.toml'
- 'README.rst'
Copy link
Member

Choose a reason for hiding this comment

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

Reminder to change this to 'README.md' once #2928 is done.

- '.github/workflows/publish-to-pypi.yml'
release:
types:
- published
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/release-drafter.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ name: Release Drafter

on:
push:
# branches to consider in the event; optional, defaults to all
branches:
- main

Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/type_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ name: Static Type Checks
on:
push:
branches: [ main ]
paths:
- 'pygmt/**/*.py'
- '.github/workflows/type_checks.yml'
pull_request:
paths:
- 'pygmt/**/*.py'
- '.github/workflows/type_checks.yml'
# Schedule daily tests
schedule:
- cron: '0 0 * * *'
Expand Down
Loading