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

Branch protection rules interfere with GitHub Actions #27

Open
pitkant opened this issue Feb 13, 2023 · 6 comments
Open

Branch protection rules interfere with GitHub Actions #27

pitkant opened this issue Feb 13, 2023 · 6 comments

Comments

@pitkant
Copy link
Member

pitkant commented Feb 13, 2023

  • @ouzor implemented branch protection rules for master branch on 6.12.2022
  • However, the requirement of making a PR first and getting reviews caused the following error message when changes to README.md were to be committed:

remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: At least 1 approving review is required by reviewers with write access.
To https://github.com/rOpenGov/helsinki
! [remote rejected] master -> master (protected branch hook declined)
error: failed to push some refs to 'https://github.com/rOpenGov/helsinki'
No changes to commit

  • Consequently I removed the branch protection rule to get the Github action working.
  • There is a GitHub Community discussion going on since 10.10.2019 about this very issue.
  • We can of course always ask whether having a separate action for rebuilding the README.md file is efficient use of computational resources and whether this should be done locally.
  • OR we can discuss on other ways on how to protect the master branch.

(Peter Evans' answer on SO was something I tried to utilise when finding ways to fix this problem but apparently it was no panacea for this specific configuration of protected branch)

@antagomir
Copy link
Member

antagomir commented Feb 13, 2023

Thanks for diving into this @pitkant !

@ouzor will decide and I am rather indifferent about the solution in this case but I also feel that README.md could be built locally, it is easy to automate anyways.

In general I think that master/main protection would be a good idea to implement also more widely for mature packages.

@ouzor
Copy link
Collaborator

ouzor commented Feb 13, 2023

Thanks @pitkant for bringing this up!
Branch protection is useful to prevent accidental pushing to master, but it's not a big deal really.
I would need to get more familiar with how the README building actually works now. Do we have similar setup in other rOpenGov repos? If so, I'm happy to follow the common conventions.

@pitkant
Copy link
Member Author

pitkant commented Feb 15, 2023

Thanks @pitkant for bringing this up!
Branch protection is useful to prevent accidental pushing to master, but it's not a big deal really.
I would need to get more familiar with how the README building actually works now. Do we have similar setup in other rOpenGov repos? If so, I'm happy to follow the common conventions.

There's some history in trying to implement a common set of GitHub Actions in the most used / active repos.

Examples of now outdated render-readme workflows with additional repository-dispatch triggers that utilise organisation level PATs to first render README and then deploy a new website:

Example of a simpler and somewhat updated workflow that fails with a cryptic error message "Process completed with exit code 1":

Example of a modern render-rmarkdown workflow that is enhanced with some additional Ubuntu system dependencies as suggested in r-lib/actions#502 and r-lib/actions#603 (it still fails with the same "Process completed with exit code 1" error message though):

I think currently the easiest workable solution, as demonstrated in this repository, would be to use @dieghernan's workflow utilising his pkgdev package (that can do other things as well than just rebuild the readme file).

Thanks for diving into this @pitkant !

@ouzor will decide and I am rather indifferent about the solution in this case but I also feel that README.md could be built locally, it is easy to automate anyways.

In general I think that master/main protection would be a good idea to implement also more widely for mature packages.

I agree that building the README files locally would probably be for the best: there's anyway some housekeeping tasks that are done locally before pushing new / updated code to GitHub and rendering README is not even the most taxing one. It would also seem that tidyverse and rOpenSci packages do not have render-rmarkdown workflows so that's another argument for ditching at least the bothersome workflows.

@antagomir
Copy link
Member

I support the suggestion from @pitkant on pkgdev package; and I also think that repeatedly fixing GHA workflows is causing more burden than rendering some of those things locally (notably, the README; potentially even the package website) and simplifying the GHA workflow this way.

It would be useful to converge to a recommended way of doing this within the rOpenGov collection.

@antagomir
Copy link
Member

Regarding the question from @ouzor the rendering happens with just knitr::knit("README.Rmd") (or rmarkdown::render("README.Rmd", output_format="md_document"). That could be added as part of script that runs the manpages with devtools::document().

@pitkant
Copy link
Member Author

pitkant commented Feb 15, 2023

Then there is the easy-to-use devtools::build_readme() (a README-specific instance of devtools::build_rmd()) that is a wrapper for rmarkdown::render

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants