-
Notifications
You must be signed in to change notification settings - Fork 18
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[PR Template Update] Updating template per identified dev process imp…
…rovements (#300) * Update PULL_REQUEST_TEMPLATE.md Updated template for making a PR according to recommendations from KK * Corrected markdown formatting Updated * Improved instructions for completion * Added link to validation workspace * Updated template according to discussion * Sections added: PR Review Checklist and Associated Documentation * Updates from developer review meeting * Finalized template after review meeting * Added word for clarity * Formatting edits * CI/CD to be completed by Theiagen dev --------- Co-authored-by: Emma Doughty <[email protected]>
- Loading branch information
1 parent
e5abd92
commit 21f7a58
Showing
1 changed file
with
72 additions
and
39 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,63 +1,96 @@ | ||
| <!-- | ||
| Thank you for contributing to Theiagen's Public Health Bioinformatics repository! | ||
| Please fill in the appropriate checklist below and delete whatever is not relevant. | ||
| Thank you for contributing to Theiagen's Public Health Bioinformatics repository! | ||
| Documentation on how to contribute can be found at https://github.com/theiagen/public_health_bioinformatics#contributing-to-the-phb-workflows | ||
| Please ensure your contributions are formatted in line with or style guide found here: https://github.com/theiagen/public_health_bioinformatics#contributing-to-the-phb-workflows and follow the instructions with <> to complete this PR. | ||
| Please replace all '[ ]' with '[X]' to demonstrate completion. | ||
| Please delete all text within '<>'. | ||
| As you create the PR, please provide all information required to validate the workflow. | ||
| --> | ||
| Closes <Issue number> | ||
|
|
||
| ## :hammer_and_wrench: Changes Being Made | ||
|
|
||
| <!--Here give examples of the changes you've made to this pull request. Include an itemized list if you can. It'll help the reviewer--> | ||
|
|
||
| ### Impacted Workflows/Tasks | ||
|
|
||
| <!--List the workflows and/or tasks that will be affected by this change--> | ||
|
|
||
| ## :brain: Context and Rationale | ||
| This PR closes #<Issue number>. | ||
|
|
||
| <!--What's the context of the changes? Were there any trade-offs you had to consider?--> | ||
| 🗑️ This dev branch should <NOT> be deleted after merging to main. | ||
|
|
||
| ## :clipboard: Workflow/Task Steps | ||
| ## :brain: Aim, Context and Functionality | ||
| <!--Please describe the aim of this PR, why the changes were made, and how the workflow should now function --> | ||
|
|
||
| <!--What are the main steps of your workflow/task? Make it explicit enough so that someone who doesn't have deep knowledge of the workflow/task can understand how the rationale was implemented.--> | ||
| ## :hammer_and_wrench: Impacted Workflows/Tasks & Changes Being Made | ||
| This will affect the behavior of the workflow(s) even if users don’t change any workflow inputs relative to the last version <!-- Delete as appropriate -->: Yes/No | ||
|
|
||
| ### Inputs | ||
| Running this workflow on different occasions could result in different results, e.g. due to use of a live database, "latest" docker image, or stochastic data processing <!-- Delete as appropriate. If yes, please describe. -->: Yes/No | ||
| <!-- | ||
| -Please use bullet points or headings to describe what is being added or modified to each impacted workflow or task, and the reasoning for those choices. | ||
| -Consider inserting before and after tables or pictures to demonstrate the consequences of the changes on files etc. | ||
| --> | ||
|
|
||
| <!--What are the mandatory and optional inputs of your workflow/task?--> | ||
| ## :clipboard: Workflow/Task Step Changes | ||
|
|
||
| ### Outputs | ||
| #### 🔄 Data Processing | ||
| <!-- How are data processed differently through the steps of the task/workflow? | ||
| Please describe in the sections below. | ||
| If nothing has changed, please explicitly say so.--> | ||
|
|
||
| <!--What are the outputs of your workflow/task?--> | ||
| Docker/software or software versions changed: | ||
|
|
||
| ### Impacted Outputs | ||
| Databases or database versions changed: | ||
|
|
||
| <!--List any existing outputs that will be affected by this change--> | ||
| Data processing/commands changed: | ||
|
|
||
| ## :test_tube: Testing | ||
| File processing changed: | ||
|
|
||
| ### Locally | ||
| Compute resources changed: | ||
|
|
||
| <!--Please show, with screenshots when possible, that your changes pass the local execution of the workflow--> | ||
| #### ➡️ Inputs | ||
| <!--Which inputs of the workflow/task have been added/removed/modified? | ||
| How have these been modified, e.g input name, type, default parameters, acceptable input ranges etc? | ||
| If nothing has changed, please explicitly say so.--> | ||
|
|
||
| ### Terra | ||
| #### ⬅️ Outputs | ||
| <!--Which outputs of the workflow/task have been added/removed/modified? | ||
| How have these been modified, e.g. output variable name, output content, output type, file changes? | ||
| If nothing has changed, please explicitly say so.--> | ||
|
|
||
| <!--Please show, with screenshots when possible and/or a URL to the job execution, that your changes pass the execution of the workflow on Terra.bio--> | ||
| ## :test_tube: Testing | ||
| #### Test Dataset | ||
| <!--Briefly describe what samples were used for testing, e.g. what organism/s, pathogen diversity, etc. --> | ||
|
|
||
| ### Scenarios for Reviewer to Test | ||
| #### Commandline Testing with MiniWDL or Cromwell (optional) | ||
| <!-- | ||
| Please show, with screenshots if possible, that your changes pass the local execution of the workflow. | ||
| If the whole test dataset was not used, please specify which samples were tested and verify the results were as anticipated. | ||
| If local testing was not undertaken/possible, please explicitly state this.--> | ||
|
|
||
| <!--Please list any potential scenarios that the reviewer should test, such as edge-cases or data types--> | ||
| #### Terra Testing | ||
| <!--Please show, with screenshots if possible and/or a URL to the job execution, that your changes pass the execution of the workflow on Terra and that all results were as anticipated (including outputs you didn't expect to change!)--> | ||
|
|
||
| ## :microscope: Quality checks | ||
| #### Suggested Scenarios for Reviewer to Test | ||
| <!--Please list any potential scenarios that the reviewer should test, including edge cases or data types--> | ||
|
|
||
| <!--Please ensure that your changes respect the following quality checks.--> | ||
| #### Theiagen Version Release Testing (optional) | ||
| <!-- | ||
| -Will changes require functional or validation testing (checking outputs etc) during the release? | ||
| -Do new samples need to be added to validation datasets? If so, upload these to the appropriate validation workspace Google bucket (). Please describe the new samples here and why these have been chosen. | ||
| -Are there any output files that should be checked after running the version release testing? | ||
| --> | ||
|
|
||
| Pull Request (PR) checklist: | ||
| - [ ] Include a description of what is in this pull request in this message. | ||
| - [ ] The workflow/task has been tested locally and on Terra | ||
| - [ ] The CI/CD has been adjusted and tests are passing | ||
| - [ ] Everything follows the [style guide](https://theiagen.notion.site/Style-Guide-WDL-Workflow-Development-bb456f34322d4f4db699d4029050481c) | ||
| ## :microscope: Final Developer Checklist | ||
| <!--Please mark boxes [X] --> | ||
| - [ ] The workflow/task has been tested locally and results, including file contents, are as anticipated | ||
| - [ ] The workflow/task has been tested on Terra and results, including file contents, are as anticipated | ||
| - [ ] The CI/CD has been adjusted and tests are passing (to be completed by Theiagen developer) | ||
| - [ ] Code changes follow the [style guide](https://theiagen.notion.site/Style-Guide-WDL-Workflow-Development-bb456f34322d4f4db699d4029050481c) | ||
|
|
||
|
|
||
| ## 🎯 Reviewer Checklist | ||
| <!-- Indicate NA when not applicable --> | ||
| - [ ] All impacted workflows/tasks have been tested on Terra with a different dataset than used for development | ||
| - [ ] All reviewer-suggested scenarios have been tested and any additional | ||
| - [ ] All changed results have been confirmed to be accurate | ||
| - [ ] All workflows/tasks impacted by change/s have been tested using a standard validation dataset to ensure no unintended change of functionality | ||
| - [ ] All code adheres to the style guide | ||
| - [ ] MD5 sums have been updated | ||
| - [ ] The PR author has addressed all comments | ||
|
|
||
| ## 🗂️ Associated Documentation (to be completed by Theiagen developer) | ||
| <!-- Indicate NA when not applicable --> | ||
| - [ ] Relevant documentation on the Public Health Resources "PHB Main" has been updated | ||
| - [ ] Workflow diagrams have been updated to reflect changes |