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

Separate workflows for running tests and building documentation #1033

Merged
merged 17 commits into from
Mar 15, 2021

Conversation

seisman
Copy link
Member

@seisman seisman commented Mar 10, 2021

Description of proposed changes

As mentioned in #830, this PR separates the current "Tests" workflow into two workflows to speedup our CI jobs.

  • ci_test.yaml: runs the full tests
  • ci_docs.yml: build the documentation and deploy it on master branch

TODO

  • Trigger workflows only if necessary (i.e., add paths and paths-ignore to push and pull_request)
  • Update MAINTENANCE.md

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@seisman seisman force-pushed the separate-workflows branch from 95002be to 98d4110 Compare March 10, 2021 21:07
@seisman seisman force-pushed the separate-workflows branch from 54d56da to 10a9a10 Compare March 10, 2021 21:43
@seisman seisman marked this pull request as ready for review March 10, 2021 22:06
@seisman seisman marked this pull request as draft March 10, 2021 22:06
@seisman seisman changed the title WIP: Separate workflows for running tests and building documentation Separate workflows for running tests and building documentation Mar 13, 2021
@seisman seisman force-pushed the separate-workflows branch from 8a77170 to 0c4b311 Compare March 15, 2021 03:08
@seisman seisman marked this pull request as ready for review March 15, 2021 03:14
@seisman seisman added the maintenance Boring but important stuff for the core devs label Mar 15, 2021
@seisman seisman added this to the 0.4.0 milestone Mar 15, 2021
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

Ok with the CI test changes, just a few comments on the inline docs.

Still can't believe that the macOS CI test can take 17min, only 2min down from 19min. That's slower than the Windows CI 14min!!!

MAINTENANCE.md Outdated Show resolved Hide resolved
MAINTENANCE.md Outdated Show resolved Hide resolved
.github/workflows/ci_tests.yaml Show resolved Hide resolved
@seisman
Copy link
Member Author

seisman commented Mar 15, 2021

Still can't believe that the macOS CI test can take 17min, only 2min down from 19min. That's slower than the Windows CI 14min!!!

The macOS CI takes only 9 minutes in the previous runs (https://github.com/GenericMappingTools/pygmt/runs/2081311267). Let's see what happens in the next runs.

Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

Please also update the master branch protection at https://github.com/GenericMappingTools/pygmt/settings/branches to include these 3 (Linux/macOS/Windows) docs build workflows.

@seisman
Copy link
Member Author

seisman commented Mar 15, 2021

One thing to note is that we're inconsistently using .yaml and .yml for the workflow files. Both work but I personally prefer .yml.

Currently, this PR uses ci_test.yaml and ci_docs.yml.

@weiji14
Copy link
Member

weiji14 commented Mar 15, 2021

One thing to note is that we're inconsistently using .yaml and .yml for the workflow files. Both work but I personally prefer .yml.

Currently, this PR uses ci_test.yaml and ci_docs.yml.

Probably best to do the renaming in a separate PR. We've are also been inconsistently using hyphens and underscores in https://github.com/GenericMappingTools/pygmt/tree/v0.3.1/.github/workflows.

Oh, and just noticed that we probably need a badge for ci_docs.yml for the main README.rst page?

@seisman
Copy link
Member Author

seisman commented Mar 15, 2021

Probably best to do the renaming in a separate PR. We've are also been inconsistently using hyphens and underscores in https://github.com/GenericMappingTools/pygmt/tree/v0.3.1/.github/workflows.

So keep using ci_docs.yml and ci_test.yaml in this PR, and rename ci_test.yaml to ci_test.yml and others in a separate PR, right?

Oh, and just noticed that we probably need a badge for ci_docs.yml for the main README.rst page?

Is it useful to you? I usually check the status of the latest commit, not the badges:
image

I feel that the badges are only useful telling new users that the package on master branch is running well and please try it, so the "Docs" build status is less useful.

@weiji14
Copy link
Member

weiji14 commented Mar 15, 2021

Probably best to do the renaming in a separate PR. We've are also been inconsistently using hyphens and underscores in https://github.com/GenericMappingTools/pygmt/tree/v0.3.1/.github/workflows.

So keep using ci_docs.yml and ci_test.yaml in this PR, and rename ci_test.yaml to ci_test.yml and others in a separate PR, right?

Yes, and we can argue about hyphens and underscores again (plus change the README badges to point to the new filenames in that PR too).

Oh, and just noticed that we probably need a badge for ci_docs.yml for the main README.rst page?

Is it useful to you? I usually check the status of the latest commit, not the badges:
image

I feel that the badges are only useful telling new users that the package on master branch is running well and please try it, so the "Docs" build status is less useful.

True, no need to add it then. All good to merge!

@seisman seisman merged commit d5b8f8a into master Mar 15, 2021
@seisman seisman deleted the separate-workflows branch March 15, 2021 04:52
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
…ricMappingTools#1033)

Separates the current "Tests" workflow into two workflows to speedup our CI jobs.

- ci_test.yaml: runs the full tests
- ci_docs.yml: build the documentation and deploy it on master branch

Co-authored-by: Wei Ji <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants