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

Only build pkgdown on version tags #450

Closed
hadley opened this issue Dec 6, 2021 · 13 comments · Fixed by #451
Closed

Only build pkgdown on version tags #450

hadley opened this issue Dec 6, 2021 · 13 comments · Fixed by #451

Comments

@hadley
Copy link
Member

hadley commented Dec 6, 2021

i.e

on:
  push:
    branches: [main, master]
    tags: ['v*']

Not all tags, as currently.

@gaborcsardi
Copy link
Member

We could also add a manual trigger, where you could specify the tag/branch to run it on, to help these cases.

@jennybc
Copy link
Member

jennybc commented Dec 6, 2021

While thinking about r-lib/pkgdown#1950, I wondered if we should use the release.published event as a trigger for pkgdown, for build & deploy of a normal release. Instead of a tag.

on:
  release:
    types: [published]

For the more rare thing where we backport changes to a released site, the workflow_dispatch event looks appropriate. That could be exposed nicely in a pkgdown and/or usethis function. If we built that into the workflow, it looks we'd be able to trigger such a run from the browser.

@hadley
Copy link
Member Author

hadley commented Dec 6, 2021

Oh yeah, release.published is likely even better.

workflow_dispatch would be useful, but I'm not sure helps with the motivating problem of rebuilding released versions.

@jennybc
Copy link
Member

jennybc commented Dec 6, 2021

workflow_dispatch would be useful, but I'm not sure helps with the motivating problem of rebuilding released versions.

I added workflow_dispatch to the on: section for usethis:

on:
  push:
    branches: [main, master]
    tags: ['*']
  workflow_dispatch:

and created a branch off the v2.1.3 release, backporting a bunch of pkgdown updates, plus an updated README (master --> main in the badges).

The branch is update-pkgdown-2-1-3.

I was able to trigger a rebuild of the released site in the browser (see usethis's gh-pages branch). You can select the "triggering" branch from the dropdown. I think it's kind of nifty 🤓 Here's the GHA landing page for usethis's pkgdown workflow.

Screen Shot 2021-12-06 at 1 28 00 PM

@hadley
Copy link
Member Author

hadley commented Dec 6, 2021

Oh hmmmm, interesting approach. It feels much better thought out than what I had in my head.

@jennybc
Copy link
Member

jennybc commented Dec 6, 2021

I guess you could still accidentally clobber a released site, if you were publishing historical releases that were missed the first time. But this feels clearly more correct than triggering for any pushed tag. And the workflow_dispatch gives a very humane way to force a pkgdown build at any state you're willing to point a branch at.

@gaborcsardi
Copy link
Member

FWIW I liked the tag (or branch) approach, because with that it is possible to fix a released pkgdown site without creating a new package release. OTOH I guess you can use the manual trigger in this case to deploy from a branch.

Should we add a parameter to the manual trigger to allow deploying from a tag?

@jennybc
Copy link
Member

jennybc commented Dec 6, 2021

Should we add a parameter to the manual trigger to allow deploying from a tag?

Or we could just keep a tag section, but with a very specific tag or tag pattern. Because run-of-the-mill releases will be handled by the release trigger.

FWIW, with workflow_dispatch, if you trigger via an API call (vs. what I did in the browser), you can specify a tag (or a branch).

https://docs.github.com/en/rest/reference/actions#create-a-workflow-dispatch-event

@hadley
Copy link
Member Author

hadley commented Dec 7, 2021

I like the idea that the released site will only be touched via a GitHub release or a manual intervention. I wonder if we should somehow connect this to the new PKGDOWN_DEV_MODE env var that lets you override the development mode implied by the version in the DESCRIPTION.

@jennybc
Copy link
Member

jennybc commented Dec 7, 2021

It is still true that you want to be checked out from a (patched?) released state, when rendering something that will overwrite the a released site. So there's no way to avoid using a branch or tag, in the case where we're travelling back in time to update a released site. (I'm not sure exactly what you're thinking re: the env var.)

@jennybc
Copy link
Member

jennybc commented Dec 7, 2021

I'll try to summarize.

on:
  push:
    branches: [main, master]
  release:
    types: [published]
  workflow_dispatch:

Keeping the push.branches part is a given.

I think the release.types part is pretty non-controversial.

I think adding workflow_dispatch hurts nothing and it immediately makes a nice browser method available for updating a released site. We could build on this with a helper function in pkgdown/usethis/devtools to trigger via an API call.

Still up for debate: whether to retain a tags section, with something considerably more specific than "*".

@gaborcsardi
Copy link
Member

I think this is all good. I would definitely omit the tags section, but we can add a tag parameter to workflow_dispatch later.

I'll merge this soon, and also add it to v1.

@github-actions
Copy link

github-actions bot commented Nov 6, 2022

This issue has been automatically locked. If you believe you have found a related problem, please file a new issue and include a link to this issue

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants