GitHub Flow Migration #330
Conversation
|
| - The `patch` branch is reserved for special hot fixes to address bugs. More info in [Hot Fix Release](release_strategy.html#hot-fix-release) | ||
| - The `main`, `devel`, `gh-pages`, `patch` branches are under protection. If you try and push changes to these branches you will get an error unless you are an administrator. |
There was a problem hiding this comment.
I think we should keep these two items and just remove devel from the second one.
There was a problem hiding this comment.
added back (but in a new PR). All comments are regarding the new PR
| @@ -43,23 +38,23 @@ Each feature branch must be related to an issue. We encourage new developers to | |||
|
|
|||
| ### Naming Branches | |||
|
|
|||
| The name of the branch must be prefixed with the issue number, followed by a short but meaningful description and the `@<target_branch>` suffix. The latter would be `@devel` in most cases. As an example, given an issue #94 "Program function to derive `LSTALVDT`", the branch name would be `94_derive_var_lstalvdt@devel`. | |||
| The name of the branch must be prefixed with the issue number, followed by a short but meaningful description and the `@<target_branch>` suffix. The latter would be `@main` in most cases. As an example, given an issue #94 "Program function to derive `LSTALVDT`", the branch name would be `94_derive_var_lstalvdt@main`. | |||
There was a problem hiding this comment.
@main does not need to be added as it is the fall back branch.
There was a problem hiding this comment.
removed the detail about main as it's not needed
|
|
||
| 1) Create a Pull Request from `devel` into the `main` branch. Issues identified in this Pull Request should have work done in separate branches and merged once again into `devel`. | ||
| 1) Verify that all CI/CD checks are passing for the `devel` into the `main` Pull Request, merge and then bundle up and send off to CRAN. See the [chapter](https://r-pkgs.org/release.html#decide-the-release-type) in R Packages for more details. | ||
| 1) Verify that all CI/CD checks and then bundle up and send off to CRAN. See the [chapter](https://r-pkgs.org/release.html#decide-the-release-type) in R Packages for more details. |
There was a problem hiding this comment.
Don't we need a PR into main were we update the version number (remove .9000 part and increase version)?
There was a problem hiding this comment.
that depends on the workflow one uses. the PR checklist from usethis, has you keep your version bump to the release version local until CRAN accepts. But others do different things.
| 1) If CRAN asks for modifications, repeat steps 1-2 as necessary. | ||
| 1) Once the package is accepted and available on CRAN, a [GitHub action](https://github.com/pharmaverse/admiral/actions/workflows/pages/pages-build-deployment) is set up to rebuild the `{admiral}` website with all the updates for this release. | ||
| 1) Use the release button on GitHub to "release" the package onto GitHub. This release onto Github archives the version of code within the `main` branch, attaches the News/Changelog file, bundles the code into a `tar.gz` file and makes a validation report via the GitHub action `validation` from [insightsengineering/validatoR](https://github.com/insightsengineering/thevalidatoR). Please see past [admiral releases](https://github.com/pharmaverse/admiral/releases) for reference and the [Releasing to Github](release_strategy.html#releasing-to-github) section for more details. | ||
| 1) Any issues fixed in the `main` branches should be merged back into `devel`. |
There was a problem hiding this comment.
Do we need a PR to main which adds the .9000 again or should this be done in the first feature branch?
There was a problem hiding this comment.
When we make releases, we should follow the checklist issue created from usethis::use_release_issue(). The last thing to do there is bump the version to xxx.9000. There is no need to wait until the first PR to increase it (although this is certainly something i have done in the past, and either way works perfectly well. But good to stick with standard practices when we can)
There was a problem hiding this comment.
Should we use use_release_issue() and follow the list created by the call or should we follow our list? Unfortunately I couldn't find an example of the list created by use_release_issue() in the usethis documentation. Most likely there are many overlaps. I think we should have just one list to follow.
| 1) Branches addressing the bugs should have Pull Requests merged into the `patch` branch **NOT** the `devel` branch. | ||
| 1) When naming the branch follow the [naming conventions](git_usage.html#implementing-an-issue) guide but use the `@main` suffix | ||
| 1) Branches addressing the bugs should have Pull Requests merged into a single `patch` branch **NOT** the `main` branch, where the `patch` branch has been created from the most recent release of the package. | ||
| 1) When naming the branch follow the [naming conventions](git_usage.html#implementing-an-issue) guide but use the `@main` suffix. |
There was a problem hiding this comment.
I think the @main suffix must not be used because it refers to the devel version of the upstream dependencies. Instead we must use the latest released version of the upstream dependencies. I am not sure how we can refer to them.
|
@bundfussr feel free to make any changes you see fit. I am less familiar with staged dependencies, and wasn't sure which notation is required there. |
|
|
||
| **Hot Fix Release**: `patch >> main >> devel` | ||
| 1) Create a GitHub release on the `patch` branch. Please see past [admiral releases](https://github.com/pharmaverse/admiral/releases) for reference and the [Releasing to Github](release_strategy.html#releasing-to-github) section for more details. | ||
| 1) These hot fixes should then be merged into the `main` branch through an additional Pull Request. |
There was a problem hiding this comment.
We need to add that the development version needs to be added again in main.
|
|
||
| 1) Identify all the bugs that need to be fixed for this hot fix release and label with hot fix label. | ||
| 1) Branches addressing the bugs should have Pull Requests merged into the `patch` branch **NOT** the `devel` branch. |
There was a problem hiding this comment.
We should add that the patch branch is updated by merging the latest released version into patch.
The devel branch here still has the release version matching the CRAN release. Needs to be updated before merging!
Thank you for your Pull Request! We have developed this task checklist from the Development Process Guide to help with the final steps of the process. Completing the below tasks helps to ensure our reviewers can maximize their time on your code as well as making sure the admiral codebase remains robust and consistent.
Please check off each taskbox as an acknowledgment that you completed the task or check off that it is not relevant to your Pull Request. This checklist is part of the Github Action workflows and the Pull Request will not be merged into the
develbranch until you have checked off each task.styler::style_file()to style R and Rmd filesdevtools::document()so all.Rdfiles in themanfolder and theNAMESPACEfile in the project root are updated appropriatelyNEWS.mdif the changes pertain to a user-facing function (i.e. it has an@exporttag) or documentation aimed at users (rather than developers)pkgdown::build_site()and check that all affected examples are displayed correctly and that all new functions occur on the "Reference" page.lintr::lint_package()R CMD checklocally and address all errors and warnings -devtools::check()