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

Enhance CI workflow triggers #3206

Merged
merged 5 commits into from Nov 4, 2022
Merged

Enhance CI workflow triggers #3206

merged 5 commits into from Nov 4, 2022

Conversation

trichter
Copy link
Member

@trichter trichter commented Nov 3, 2022

What does this PR do?

I tried to improve the workflow triggers. The goal was to trigger the corresponding workflows when specific labels are added using the "labeled" pull_request event. This was impractical, because workflows were triggered for all new applied labels. Later, jobs can be canceled for inappropriate labels, but then the checks tab will only show these skipped jobs.
Therefore, I removed this feature again for the docs_building workflow.

Secondly, I added a trigger for the wheel building; these are now triggered in PRs with the test_wheels label instead of using specific branch names.

This PR is branched from #3205.

PR Checklist

  • Correct base branch selected? master for new features, maintenance_... for bug fixes
  • This PR is not directly related to an existing issue (which has no PR yet).
  • If the PR is making changes to documentation, docs pages can be built automatically.
    Just add the "build_docs" tag to this PR.
    Docs will be served at docs.obspy.org/pr/{branch_name} (do not use master branch).
    Please post a link to the relevant piece of documentation.
  • If all tests including network modules (e.g. clients.fdsn) should be tested for the PR,
    just add the "test_network" tag to this PR.
  • All tests still pass.
  • Any new features or fixed regressions are covered via new tests.
  • Any new or changed features are fully documented.
  • Significant changes have been added to CHANGELOG.txt .
  • First time contributors have added your name to CONTRIBUTORS.txt .
  • If the changes affect any plotting functions you have checked that the plots
    from all the CI builds look correct. Add the "upload_plots" tag so that plotting
    outputs are attached as artifacts.
  • Add the "ready for review" tag when you are ready for the PR to be reviewed.

@trichter trichter added build_docs Docs will be automatically built and deployed in github actions on pushes to the PR CI issue generally related to continuous integration test_network tell github actions to also run network tests for this PR bug-unconfirmed reported bug that still needs to be confirmed bug confirmed bug and removed build_docs Docs will be automatically built and deployed in github actions on pushes to the PR test_network tell github actions to also run network tests for this PR bug-unconfirmed reported bug that still needs to be confirmed bug confirmed bug CI issue generally related to continuous integration labels Nov 3, 2022
@trichter trichter added test_wheels and removed build_docs Docs will be automatically built and deployed in github actions on pushes to the PR test_network tell github actions to also run network tests for this PR labels Nov 3, 2022
@trichter trichter added the ready for review PRs that are ready to be reviewed to get marked ready to merge label Nov 4, 2022
@trichter trichter added this to the 1.4.0 milestone Nov 4, 2022
@megies
Copy link
Member

megies commented Nov 4, 2022

Therefore, I removed this feature again for the docs_building workflow.

Hmm.. so docs building will only be triggered on commit pushes again, after the label was added?

Secondly, I added a trigger for the wheel building; these are now triggered in PRs with the test_wheels label instead of using specific branch names.

Nice! Wanted to propose that earlier, but then didn't to not add more workload

@trichter
Copy link
Member Author

trichter commented Nov 4, 2022

Hmm.. so docs building will only be triggered on commit pushes again, after the label was added?

Both ways have disadvantages.

At master, the docs bulding is altogether canceled if another label is added and if the worker is still running. This of course can be enhanced. But still, we cannot trigger the workflow only when the build_docs label is added. It is only possible to trigger the workflow for all labels and only after that skip the job if a label different form build_docs was applied. The checks tabs only displays the latest run of a workflow and would display docs / build (pull_request) Skipped in that case. Quite confusing everything. IMHO it is better to push an (empty) commit. Then all three ci labels (build_docs, test_network, test_wheels, and unrelated here: no_ci) are similarly handled.

@trichter
Copy link
Member Author

trichter commented Nov 4, 2022

OK, maybe I can add that back for the docs label.

@megies
Copy link
Member

megies commented Nov 4, 2022

At master, the docs bulding is altogether canceled if another label is added

That I don't understand.. labels can only be on PRs, no?

.. and if the worker is still running. This of course can be enhanced.

Yea, I guess you cannot prevent docs build on master to get interrupted on master pushes, but I dont think its a problem is it? Docs building is really fast these days, and we don't push to master that often and at least over night (/ quieter hours) CI will be able to build new docs uninterrupted by pushes.

But still, we cannot trigger the workflow only when the build_docs label is added. It is only possible to trigger the workflow for all labels and only after that skip the job if a label different form build_docs was applied. The checks tabs only displays the latest run of a workflow and would display docs / build (pull_request) Skipped in that case. Quite confusing everything.

Hmm yea.. thats quite unfortunate

IMHO it is better to push an (empty) commit. Then all three ci labels (build_docs, test_network, test_wheels, and unrelated here: no_ci) are similarly handled.

Sure, we can go back to that, no problem with that on my end, given the above caveats

Copy link
Member

@megies megies left a comment

Choose a reason for hiding this comment

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

change as you like, maybe PR template needs to be adjusted though, to point out how docs builds can be triggered (or not)

@trichter trichter added build_docs Docs will be automatically built and deployed in github actions on pushes to the PR and removed test_wheels labels Nov 4, 2022
@trichter trichter added ready for review PRs that are ready to be reviewed to get marked ready to merge build_docs Docs will be automatically built and deployed in github actions on pushes to the PR and removed ready for review PRs that are ready to be reviewed to get marked ready to merge build_docs Docs will be automatically built and deployed in github actions on pushes to the PR labels Nov 4, 2022
@trichter trichter merged commit c02976c into master Nov 4, 2022
@trichter trichter deleted the actions branch November 4, 2022 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build_docs Docs will be automatically built and deployed in github actions on pushes to the PR CI issue generally related to continuous integration ready for review PRs that are ready to be reviewed to get marked ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants