Skip to content

Conversation

@matthewfeickert
Copy link
Member

@matthewfeickert matthewfeickert commented Aug 26, 2022

Description

Split the docs workflow into a build and deploy jobs, where the deploy job requires the build job to succeed. This avoids unnecessary deployments by only having the github-pages environment exist in the deploy job.

Checklist Before Requesting Reviewer

  • Tests are passing
  • "WIP" removed from the title of the pull request
  • Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

  • Summarize commit messages into a comprehensive review of the PR
* Split the docs workflow into a build and deploy jobs, where the deploy
  job requires the build job to succeed. This avoids unnecessary deployments
  by only having the github-pages environment exist in the deploy job.
* Constrain the deployment of environments to PR merges by requiring push
  events on master to trigger the deploy job.

Co-authored-by: Giordon Stark <[email protected]>

* The actions/upload-pages-artifacts action deploys an environment
  so to avoid unnecessary deployments only have it run on pushes to
  master which occur on PR merges.
@matthewfeickert matthewfeickert added CI CI systems, GitHub Actions fix A bug fix labels Aug 26, 2022
@matthewfeickert matthewfeickert requested a review from kratsg August 26, 2022 15:41
@matthewfeickert matthewfeickert self-assigned this Aug 26, 2022
@matthewfeickert matthewfeickert temporarily deployed to github-pages August 26, 2022 15:42 Inactive
@matthewfeickert matthewfeickert marked this pull request as draft August 26, 2022 15:49
@matthewfeickert
Copy link
Member Author

Hm. @kratsg this isn't having the intended effect with deployment history.

Screenshot from 2022-08-26 10-49-00

@codecov
Copy link

codecov bot commented Aug 26, 2022

Codecov Report

Merging #1960 (50f5e11) into master (868c27d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1960   +/-   ##
=======================================
  Coverage   98.24%   98.24%           
=======================================
  Files          68       68           
  Lines        4380     4380           
  Branches      728      728           
=======================================
  Hits         4303     4303           
  Misses         45       45           
  Partials       32       32           
Flag Coverage Δ
contrib 26.55% <ø> (ø)
doctest 60.54% <ø> (ø)
unittests-3.10 96.14% <ø> (ø)
unittests-3.7 96.12% <ø> (ø)
unittests-3.8 96.16% <ø> (ø)
unittests-3.9 96.18% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@kratsg kratsg temporarily deployed to github-pages August 26, 2022 18:32 Inactive
@kratsg kratsg temporarily deployed to github-pages August 26, 2022 22:56 Inactive
@kratsg kratsg temporarily deployed to github-pages August 27, 2022 03:05 Inactive
@kratsg
Copy link
Contributor

kratsg commented Aug 27, 2022

Is this still a draft?

@matthewfeickert
Copy link
Member Author

Is this still a draft?

Yes, as this isn't having the intended effect. c.f. #1960 (comment)

@kratsg kratsg temporarily deployed to github-pages August 27, 2022 03:21 Inactive
@kratsg
Copy link
Contributor

kratsg commented Aug 27, 2022

Yes, as this isn't having the intended effect. c.f. #1960 (comment)

It should be fixed now. I moved the environment + permissions to a deploy job and that job only runs on master. Pages artifacts still uploads, but now the environment isn't being created or deployed to.

@matthewfeickert matthewfeickert marked this pull request as ready for review August 27, 2022 06:02
@matthewfeickert matthewfeickert merged commit bfd5a2a into master Aug 27, 2022
@matthewfeickert matthewfeickert deleted the ci/only-deploy-on-main branch August 27, 2022 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI CI systems, GitHub Actions fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants