GitHub flow (#349)

* docs: #283 update description and news * closes #286 commit messaging, new r-cmd vignette (#291) * docs: #286 commit messaging, new r-cmd vignette * chore: #286 spelling * docs: #286 squash and merge blurb * Update vignettes/git_usage.Rmd Co-authored-by: Zelos Zhu <[email protected]> * chore: #286 news update --------- Co-authored-by: Zelos Zhu <[email protected]> * Closes #271, #213, #260, #240 Documentation Update of get_datasets(); keep_source_vars argument; @family tag; compute_ functions (#287) * docs: #271 #213 clarify get_datasets and add keep_source_vars to prog strat * docs: #260 cleanup @family mentions * docs: #240 add blurb about compute functions * docs: #271 #213 #240 #260 add news blurb * adopt suggestion for family/keywords Co-authored-by: Ben Straub <[email protected]> * docs #271 adopt get_dataset feedback * docs: #213 #240 adopt recommendations from PR * chore: #240 add BMI to WORDLIST for spellcheck * chore: #271 #213 #260 adopt feedback --------- Co-authored-by: Zelos Zhu <[email protected]> Co-authored-by: Ben Straub <[email protected]> * Closes #264, #288 cleanup assertions and continue deprecation process (#289) * feat: #288 cleanup deprecation process * feat: #264 deprecate assert_function * test: #264 cleanup some tests in assert_function, confused how others would go * test: #264 rewrite tests in a more logical way * chore: #264 run styler/lintr * chore: #288 revert back to devel test file * chore: #288 properly fix assert_order_vars again * feat: #264 begin deprecation of redundant assertions * feat: #288 deprecate assert_named_exprs * deprecate assert_has_variables properly #264 * chore: #264 run styler * docs: #264 add blurb in news for the deprecated assertions * chore: #264 fix description version and remove deprecated examples --------- Co-authored-by: Zelos Zhu <[email protected]> * Propagate renv.lock from pharmaverse/admiralci (#294) renv update from pharmaverse/admiralci Co-authored-by: dgrassellyb <[email protected]> * Closes #22 #181 #201 #292 #298 Variety of small-scale general documentation updates (#303) * feat: #22 add documentation to friendly_type_of * feat: #181 add the appropriate URL * feat: #201 #292 add documentation about PR guidance and codeowners * chore: update wordlist for codeowners * chore: add NEWS --------- Co-authored-by: Zelos Zhu <[email protected]> * Closes #302 Adding Snapshot testing guidance to unit testing vignette (#308) * Update unit_test_guidance.Rmd * Update NEWS.md * Update NEWS.md * Closes #301: (#307) * #301: edoardo added as author, and author/contributor distinction implemented as in core admiral package * #301 chore: document * Propagate renv.lock from pharmaverse/admiralci (#310) * renv / codespaces update from pharmaverse/admiralci --------- Co-authored-by: galachad <[email protected]> Co-authored-by: Adam Foryś <[email protected]> * Closes #295 template documentation@devel (#300) * #295 set up package extension guidance page * #295 fix vignetteindexentry * #295 chore: spellcheck * #295 chore: fix encoding issues in vignettes that prevented package from being built * #295: updates following review * #295 Chore: spellcheck * Update vignettes/package_extensions.Rmd Co-authored-by: Ross Farrugia <[email protected]> * #295 update to refer to admiraldev devel site * #295 Chore: spellcheck --------- Co-authored-by: Ross Farrugia <[email protected]> * Closes #296 document_missing_value_s@devel (#311) * #296 - `missing_value` and `missing_values` added to the table of common arguments. * #296 - add in discussion of when to use singular vs plural arguments to programming strategy. * #296 - run checks required for PR and update NEWS.md. * #296 - Update according to requested changes. * #296 - revert the 'wrapped' changes in the programming strategy vignette * #296 - re-insert the new text into the programming strategy vignette, altered according to changes requested. * #296 - update NEWS.md with changes requested and run the required checks for PR. * Closes #282: Test Data Guidance vignette (#293) * #282: add test_data_guidance.Rmd, copy from admiraldata README devel * #282: update .yml * #282: add link for packages, remove SDTM, add naming conventions for program name * #282 spelling * #282: link to two data packages * #282: updated wordlist * #282: update pharmaverseadam description --------- Co-authored-by: Edoardo Mancini <[email protected]> * Propagate renv.lock from pharmaverse/admiralci (#314) renv / codespaces update from pharmaverse/admiralci Co-authored-by: galachad <[email protected]> * Propagate renv.lock from pharmaverse/admiralci (#315) * renv / codespaces update from pharmaverse/admiralci * renv / codespaces update from pharmaverse/admiralci --------- Co-authored-by: galachad <[email protected]> * Closes #306 argument descriptions added to table (#320) argument descriptions added to table Co-authored-by: Zelos Zhu <[email protected]> * Closes #316 remove messaging that includes "-" as year not handled (#317) * feat: #316 remove messaging that includes "-" as year not handled * feat: #316 replace warning message * chore: #316 fix warning message and typos * chore: #316 add news blurb --------- Co-authored-by: Zelos Zhu <[email protected]> * Closes #318 #321 Documentation updates around admiral.test, staged dependencies, and function arguments (#323) * Fix hardcoded URL (#326) Resolves #325 * Closes #312 breakup wall of text (#319) * brick in the wall * remove special characters from image name * remove special characters from image name * replaced iframe with png * fixed image reference --------- Co-authored-by: Zelos Zhu <[email protected]> * Closes #328 add missing news entries (#329) * feat: #295 add missing news entries * feat: #295 add missing news entries * fixed it * feat: add news entry for #306 * chore: #312 missing reference * chore: fix link that pointed to merged branch and use devel * Update NEWS.md --------- Co-authored-by: Zelos Zhu <[email protected]> Co-authored-by: Ben Straub <[email protected]> * GitFlow Migration * rename to match repo/pkg name * Update release_strategy.Rmd * progress * chore: branch image and main * chore: images updated to our modern times * docs: new images, removed most of the action sections * chore: delete dev process * chore: new blurb for the people --------- Co-authored-by: Zelos Zhu <[email protected]> Co-authored-by: Zelos Zhu <[email protected]> Co-authored-by: Ben Straub <[email protected]> Co-authored-by: pharmaverse-bot <[email protected]> Co-authored-by: dgrassellyb <[email protected]> Co-authored-by: Edoardo Mancini <[email protected]> Co-authored-by: galachad <[email protected]> Co-authored-by: Adam Foryś <[email protected]> Co-authored-by: Ross Farrugia <[email protected]> Co-authored-by: Sophie Shapcott <[email protected]> Co-authored-by: Kangjie Zhang <[email protected]> Co-authored-by: StefanThoma <[email protected]> Co-authored-by: cicdguy <[email protected]>
pharmaverse · Nov 17, 2023 · 76fee62 · 76fee62
1 parent 4a66aa5
commit 76fee62
Show file tree

Hide file tree

Showing 12 changed files with 39 additions and 101 deletions.
diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
@@ -1,6 +1,6 @@
 Thank you for your Pull Request! We have developed this task checklist from the [Development Process Guide](https://pharmaverse.github.io/admiraldev/articles/development_process.html) 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 `devel` branch until you have checked off each task.
+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 `main` branch until you have checked off each task.
 
 - [ ] Place Closes #<insert_issue_number> into the beginning of your Pull Request Title (Use Edit button in top-right if you need to update)
 - [ ] Code is formatted according to the [tidyverse style guide](https://style.tidyverse.org/). Run `styler::style_file()` to style R and Rmd files

diff --git a/NEWS.md b/NEWS.md
@@ -17,6 +17,7 @@
 
 - New documentation in programming strategy around quoting/expressions and standardizing roxygen texts (#233, #332)
 - New documentation on how to use footnotes when writing vignettes (#324)
+- Updated language and iamges to adopt GitHub Flow Strategy (#349)
 
 ## Various
 

diff --git a/vignettes/git_usage.Rmd b/vignettes/git_usage.Rmd
@@ -24,14 +24,12 @@ This article will give you an overview of how the `{admiral}` project is utilizi
 
 # Branches
 
--	The `main` branch contains the latest **released** version and should not be used for development. You can find the released versions [here](https://GitHub.com/pharmaverse/admiral/releases) 
--	The `devel` branch contains the latest development version of the package. You will always be branching off and pulling into the `devel` branch. This is set as the default branch on GitHub.
-- The `gh-pages` branches contains the code used to render this website you are looking at right now!
-- 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. 
-
--	**Feature** branches are where actual development related to a specific issue happens. Feature branches are merged into `devel` once a pull request is merged. Check out the [Pull Request Review Guidance](pr_review_guidance.html) for more guidance on merging into `devel`.
+-	The `main` branch contains the latest development version of the package. You can find the released versions [here](https://GitHub.com/pharmaverse/admiral/releases) 
+- The `gh-pages` branches contains the code used to render R package websites - you are looking at right now!
+- The `patch` branch is reserved for special hot fixes to address bugs and should rarely be used. More info in [Hot Fix Release](release_strategy.html#hot-fix-release)
+- The `main`, `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. 
 
+-	**Feature** branches are where actual development related to a specific issue happens. Feature branches are merged into `main` once a pull request is merged. Check out the [Pull Request Review Guidance](pr_review_guidance.html) for more guidance on merging into `main`.
 
 # Working with Feature Branches
 
@@ -43,23 +41,21 @@ 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 `@<target_branch>` suffix is used in our CI/CD pipelines, e.g. when running `R CMD check`. It ensures that `{admiral}`'s dependencies such as `{pharmaversesdtm}` and `{admiraldev}` are installed from the specified target branch. So when the target branch is set to `@devel` the dependencies will be installed from those package's respective `devel` branches rather than installing the latest released version. This ensures that we test the development version of `{admiral}` against the development versions of its  dependencies. That way all packages are kept in sync.
+The name of the branch must be prefixed with the issue number, followed by a short but meaningful description. As an example, given an issue #94 "Program function to derive `LSTALVDT`", the branch name would be `94_derive_var_lstalvdt`.
 
-### Create a New Feature Branch from the Terminal (from `devel`)
+### Create a New Feature Branch from the Terminal (from `main`)
 
--	Checkout the devel branch: `git checkout devel`  
+-	Checkout the main branch: `git checkout main`  
 -	Pull the latest changes from GitHub: `git pull`  
--	Create a new branch off the devel branch and switch to it: `git checkout -b <new_branch_name>`
+-	Create a new branch off the main branch and switch to it: `git checkout -b <new_branch_name>`
 
-### Create a New Feature Branch from GitHub (from `devel`)
+### Create a New Feature Branch from GitHub (from `main`)
 
 You can also create a feature branch in GitHub. 
 
-- Switch to the `devel` branch
+- Switch to the `main` branch
 - Type in your new feature branch name
-- Click Create branch: `<your_branch_name>@devel` from `devel`
+- Click Create branch: `<your_branch_name>@main` from `main`
 - Be Sure to Pull down newly created branch into RStudio
 
 ```{r, echo = FALSE}
@@ -113,7 +109,12 @@ We recommend a thorough read through of the articles, [Pull Request Review Guida
 Once all changes are committed, push the updated branch to GitHub:  
 `git push -u origin <branch_name>`  
 
-In GitHub, under **Pull requests**, the user will either have a "Compare and pull request" button and/or a "Create Pull Request".  The first button will be created for you if GitHub detects recent changes you have made. The branch to merge with must be the `devel` branch (base = `devel`) and the compare branch is the new branch to merge - as shown in the below picture. Please **pay close attention** to the branch you are merging into!  
+In GitHub, under **Pull requests**, the user will either have a "Compare and pull request" button and/or a "Create Pull Request".  The first button will be created for you if GitHub detects recent changes you have made. The branch to merge with must be the `main` branch (base = `main`) and the compare branch is the new branch to merge - as shown in the below picture. Please **pay close attention** to the branch you are merging into!  
+
+
+```{r, echo = FALSE}
+knitr::include_graphics("github_create_pr.png", dpi = 144)
+```
 
 The issue must be linked to the pull request in the "Development" field of the
 Pull Request. In most cases, this will linkage will automatically close the issue and move to the Done column on our project board.
@@ -124,10 +125,6 @@ knitr::include_graphics("github_linked_issues_dark.png", dpi = 144)
 
 Once you have completed the Pull Request you will see all committed changes are then available for the reviewer.  A reviewer must be specified in the Pull Request. It is recommended to write a brief summary to your reviewers so they can quickly come up to speed on your Pull Request.  Images of your updates are nice too, which are easy to do in GitHub!  Use any Screen Capture software and Copy and Paste into your summary. 
 
-```{r, echo = FALSE}
-knitr::include_graphics("github_create_pr.png", dpi = 144)
-```
-
 ### Reviewing/Closing an Issue
 
 - At least one reviewer must approve the Pull Request. Please review the [Pull
@@ -153,10 +150,10 @@ knitr::include_graphics("github_done.png", dpi = 144)
 Merge conflict is a situation where `git` cannot decide which changes to apply since there were multiple updates in the same part of a file. This typically happens when multiple people update the same part of code. Those conflicts always need to be handled manually (as some further code updates may be required):
 
 ```
-git checkout devel
+git checkout main
 git pull
 git checkout <feature_branch>  
-git merge devel
+git merge main
 ```
 
 This provides a list of all files with conflicts In the file with conflicts the conflicting sections are marked with `<<<<<<<`, `=======`, and `>>>>>>>`. The code between these markers must be updated and the markers be removed. Source files need to be updated manually. Generated files like NAMESPACE or the generated documentation files should not be updated manually but recreated after the source files were updated. 

diff --git a/vignettes/github_create_pr.png b/vignettes/github_create_pr.png
diff --git a/vignettes/github_done.png b/vignettes/github_done.png
diff --git a/vignettes/github_feature_branch.png b/vignettes/github_feature_branch.png
diff --git a/vignettes/github_linked_issues_dark.png b/vignettes/github_linked_issues_dark.png
diff --git a/vignettes/package_extensions.Rmd b/vignettes/package_extensions.Rmd
@@ -57,6 +57,7 @@ _Note: The ordering numbers below are suggested but don't all need to strictly b
 1. Agree on a charter and expectations of each company, e.g. we usually ask for at least 3 developers with at least 25% capacity and a mix of R, GitHub and TA experience. Within the charter make sure the scope and timelines are clear. _It is important here not to try to boil the ocean. Focus first on the very common endpoints required as a foundation and then the package can build up from here via contributions from both the co-development companies and also the wider across-industry admiral community. If useful, the `{admiralonco}` charter could be shared as a guide._
 
 1. Each company should start to identify the required developer resources. Then each developer is required to complete the `{admiral`} [dummy issue for onboarding](https://github.com/pharmaverse/admiral/issues/1839), as well as reading up on the [admiraldev documentation](https://pharmaverse.github.io/admiraldev/index.html), - especially the developer guides, which all need to be followed for package extensions.
+
 ![Dummy issue for new developers](https://github.com/pharmaverse/admiraldev/raw/main/vignettes/dummy_issue.png){width=100%}
 
 1. Optionally it can be useful to host a kick-off meeting to decide how the team will work, for which we recommend agile/scrum practices.

diff --git a/vignettes/pr_review_checkbox.png b/vignettes/pr_review_checkbox.png
diff --git a/vignettes/pr_review_checklist.png b/vignettes/pr_review_checklist.png
diff --git a/vignettes/pr_review_guidance.Rmd b/vignettes/pr_review_guidance.Rmd
@@ -21,13 +21,13 @@ knitr::opts_chunk$set(
 
 This document is intended to be guidance for creators and reviewers of pull requests (PRs) in the `{admiral}` package family. PR authors will benefit from shorter review times by closely following the guidance provided here.
 
-A pull request into the `devel` branch signifies that an issue has been "addressed". This issue might be a bug, a feature request or a documentation update. Once a Pull Request is merged into `devel` branch, then the issue(s) can be closed.
+A pull request into the `main` branch signifies that an issue has been "addressed". This issue might be a bug, a feature request or a documentation update. Once a Pull Request is merged into `main` branch, then the issue(s) can be closed.
 
-Closely following the below guidance will ensure that our all our "addressed" issues auto-close once we merge `devel`.
+Closely following the below guidance will ensure that our all our "addressed" issues auto-close once we merge `main`.
 
 # Review Criteria
 
-For a pull request to be merged into `devel` it needs to pass the automated CI checks that will appear at the bottom of the Pull Request. In addition, the PR creator and reviewer should make sure that
+For a pull request to be merged into `main` it needs to pass the automated CI checks that will appear at the bottom of the Pull Request. In addition, the PR creator and reviewer should make sure that
 
 -   the [Programming Strategy](programming_strategy.html) and [Development Process](https://pharmaverse.github.io/admiral/articles/contribution_model.html) are followed
 
@@ -83,76 +83,20 @@ knitr::include_graphics("./pr_review_checkbox.png")
 
 ## Complete the Pull Request checklist
 
-The check boxes are linked to the `task-list-completed` workflow. You need to check off each box in acknowledgment that you have done you due diligence in creating a compliant Pull Request. GitHub will refresh the Pull Request and trigger `task-list-completed` workflow that you have completed the task. The PR can not be merged into `devel` until the contributor has checked off each of the check box items.
+The check boxes are linked to the `task-list-completed` workflow. You need to check off each box in acknowledgment that you have done you due diligence in creating a compliant Pull Request. GitHub will refresh the Pull Request and trigger `task-list-completed` workflow that you have completed the task. The PR can not be merged into `main` until the contributor has checked off each of the check box items.
 
 ```{r echo=FALSE, out.width='120%'}
 knitr::include_graphics("./pr_review_actions.png")
 ```
 
 Please don't hesitate to reach out to the `{admiral}` team on [Slack](https://app.slack.com/client/T028PB489D3/C02M8KN8269) or through the [GitHub Issues](https://github.com/pharmaverse/admiral/issues) tracker if you think this checklist needs to be amended or further clarity is needed on a check box item.
 
-**Note for Reviewers:** We recommend the use of Squash and Merge when merging in a Pull Request. This will create a clean commit history when doing a final merge of `devel` into `main`.
+**Note for Reviewers:** We recommend the use of Squash and Merge when merging in a Pull Request. This will create a clean commit history.
 
 # GitHub Actions/CI Workflows
 
 The `task-list-completed` workflow is one of the several workflows/actions used within `{admiral}`. These workflows live in the `.github/workflows` folder and it is important to understand their use and how to remedy if the workflow fails. Workflows defined here are responsible for assuring high package quality standards without compromising performance, security, or reproducibility.
 
 ## A synopsis of admiral's workflows
 
-Most workflows have a `BEGIN boilerplate steps` and `END boilerplate steps` section within them which define some standard steps required for installing system dependencies, R version and R packages which serve as dependencies for the package.
-
-The underlying mechanisms for installing R and Pandoc are defined in `r-lib/actions`, while the installation of system dependencies and R package dependencies is managed via the Staged Dependencies GitHub Action]. The latter is used in conjunction with the `staged_dependencies.yaml` file in order to install dependencies that are in the *same stage of development* as the current package.
-
-Following the installation of system dependencies, R, and package dependencies, each workflow checks the integrity of a specific component of the admiral codebase.
-
-### `check-templates.yml`
-
-This workflow checks for issues within template scripts. For example, in the admiral package there are several template scripts with admiral-based functions showing how to build certain ADaM datasets. As we update the admiral functions, we want to make sure these template scripts execute appropriately. Functions in the template scripts that are deprecated or used inappropriately will cause this workflow to fail. Click on the details button on a failing action provides information on the where the template is failing.
-
-### `code-coverage.yml`
-
-This workflow measures code coverage for unit tests and reports the code coverage as a percentage of the *total number of lines covered by unit tests* vs. the *total number of lines in the codebase*.
-
-The `{covr}` R package is used to calculate the coverage.
-
-Report summaries and badges for coverage are generated using a series of other GitHub Actions.
-
-### `links.yml`
-
-This workflow checks whether URLs embedded in code and documentation are valid. Invalid URLs results in workflow failures. This workflow uses `lychee` to detect broken links. Occasionally this check will detect false positives of urls that look like urls. To remedy, please add this false positive to the `.lycheeignore` file.
-
-### `lintr.yml`
-
-Static code analysis is performed by this workflow, which in turn uses the `{lintr}` R package. The `.lintr` configurations in the repository will be by this workflow.
-
-### `man-pages.yml`
-
-This workflow checks if the manual pages in the `man/` directory of the package are up-to-date with ROxygen comments in the code.
-
-Workflow failures indicate that the manual pages are not up-to-date with ROxygen comments, and corrective actions are provided in the workflow log.
-
-### `pkgdown.yml`
-
-Documentation for the R package is generated via this workflow. This workflow uses the `{pkgdown}` framework to generate documentation in HTML, and the HTML pages are deployed to the `gh-pages` branch.
-
-Moreover, an additional `Versions` dropdown is generated via the `multi-version-docs` GitHub Action, so that an end user can view multiple versions of the documentation for the package.
-
-### `r-cmd-check.yml`
-
-This workflow performs `R CMD check` for the package. Failed workflows are typically indicative of problems encountered during the check, and therefore an indication that the package does not meet quality standards.
-
-### `r-pkg-validation.yml`
-
-When a new release of the package is made, this workflow executes to create a validation report via `validation` action. The PDF report is then attached to the release within GitHub.
-
-### `readme-render.yml`
-
-If your codebase uses a [`README.Rmd` file](../../README.Rmd), then this workflow will automatically render a `README.md` and commit it to your branch.
-
-### `spellcheck.yml`
-
-Spellchecks are performed by this workflow, and the `{spelling}` R package is used to detect spelling mistakes. Failed workflows typically indicate misspelled words. In the `inst/WORDLIST` file, you can add words and or acronyms that you want the spell check to ignore, for example occds is not an English word but a common acronym used within Pharma. The workflow will flag this until a user adds it to the `inst/WORDLIST`.
-
-### `style.yml`
-
-Code style is enforced via the `styler` R package. Custom style configurations, if any, will be honored by this workflow. Failed workflows are indicative of unstyled code.
+We recommend checking out the [README](https://github.com/pharmaverse/admiralci#cicd-workflows-for-admiral-r-packages) on the admiralci repository to gain an understanding of the workflows/actions used in `{admiral}`.