From a3d8ba028ef7c784778383cfbb21956a9d361366 Mon Sep 17 00:00:00 2001 From: PawelPlesniak Date: Tue, 11 Nov 2025 15:08:17 +0100 Subject: [PATCH 01/14] Making testing requirements clearer --- .github/PULL_REQUEST_TEMPLATE.md | 25 ------------------ .github/pull_request_template.md | 45 +++++++++++++++++++++----------- 2 files changed, 30 insertions(+), 40 deletions(-) delete mode 100644 .github/PULL_REQUEST_TEMPLATE.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md deleted file mode 100644 index 1bfa823db..000000000 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ /dev/null @@ -1,25 +0,0 @@ -# Description -Fixes # (issue_number) - -_Please include a summary of the change and which issue is fixed (if any). Please also -include relevant motivation and context. List any dependencies that are required for -this change._ - -## Type of change - -- [ ] Documentation (non-breaking change that adds or improves the documentation) -- [ ] New feature (non-breaking change which adds functionality) -- [ ] Optimization (non-breaking, back-end change that speeds up the code) -- [ ] Bug fix (non-breaking change which fixes an issue) -- [ ] Breaking change (whatever its nature) - -## Key checklist - -- [ ] All tests pass (eg. `python -m pytest`) -- [ ] Pre-commit hooks run successfully (eg. `pre-commit run --all-files`) - -## Further checks - -- [ ] Code is commented, particularly in hard-to-understand areas -- [ ] Tests added or an issue has been opened to tackle that in the future. - (Indicate issue here: # (issue)) \ No newline at end of file diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index abab46928..914eba286 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,26 +1,41 @@ # Description + +_If full decription and testing details are included on a parent issue, please link to that here._ +See issue # for details + +_Otherwise, please include a summary of the change and which issue is fixed (if any). +Include relevant motivation and context, including a target environment and dunedaq version if known. +Also list any dependencies that are required for this change._ Addresses issue # -_Please include a summary of the change and which issue is fixed (if any). Please also -include relevant motivation and context. List any dependencies that are required for -this change._ -Fixes # (issue) +_Please also include instructions for how a reviewer can test your changes._ + +## Requires + +If applicable, list all the related PRs required to implement and test this change. + +## Testing checklist + +Testing was ran on: _include the host name here_ + +- [ ] Unit tests pass (`pytest`) +- [ ] Minimal system quicktest passes (`pytest -s minimal_system_quick_test.py`) +- [ ] All integration tests pass (`daqsystemtest_integtest_bundle.sh`) +- [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) +- [ ] Testing skipped as this is a documentation PR + ## Type of change -- [ ] Documentation (non-breaking change that adds or improves the documentation) -- [ ] New feature (non-breaking change which adds functionality) -- [ ] Optimization (non-breaking, back-end change that speeds up the code) +- [ ] New feature or enhancement (non-breaking change which adds functionality) +- [ ] Optimization (non-breaking change that improves code/performance) - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] Breaking change (whatever its nature) - -## Key checklist - -- [ ] All tests pass (eg. `python -m pytest`) -- [ ] Pre-commit hooks run successfully (eg. `pre-commit run --all-files`) +- [ ] Documentation (non-breaking change that adds or improves the documentation) ## Further checks -- [ ] Code is commented, particularly in hard-to-understand areas -- [ ] Tests added or an issue has been opened to tackle that in the future. - (Indicate issue here: # (issue)) \ No newline at end of file +- [ ] Code is commented where needed, particularly in hard-to-understand areas +- [ ] Code style is correct (`dbt-build --lint`, and/or see https://dune-daq-sw.readthedocs.io/en/latest/packages/styleguide/) +- [ ] If applicable, new tests have been added or an issue has been opened to tackle that in the future. + (Indicate issue here: # (issue)) From 9b0e7e33a064ca3b7db22108de0443cbb359ca17 Mon Sep 17 00:00:00 2001 From: PawelPlesniak Date: Thu, 20 Nov 2025 22:17:53 +0100 Subject: [PATCH 02/14] Adding the remainding tests --- .github/pull_request_template.md | 37 +++++++++++++++++--------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 914eba286..8085edcfd 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,37 +1,40 @@ # Description -_If full decription and testing details are included on a parent issue, please link to that here._ -See issue # for details - -_Otherwise, please include a summary of the change and which issue is fixed (if any). -Include relevant motivation and context, including a target environment and dunedaq version if known. -Also list any dependencies that are required for this change._ Addresses issue # +_This issue should contain the full description of what the PR is intended to deliver._ + +## Type of change -_Please also include instructions for how a reviewer can test your changes._ +- [ ] New feature or enhancement (non-breaking change which adds functionality) +- [ ] Optimization (non-breaking change that improves code/performance) +- [ ] Bug fix (non-breaking change which fixes an issue) +- [ ] Breaking change (whatever its nature) +- [ ] Documentation (non-breaking change that adds or improves the documentation) ## Requires If applicable, list all the related PRs required to implement and test this change. +## Changelog + +_Include a high level overview of the changes introduced by this PR._ + ## Testing checklist Testing was ran on: _include the host name here_ -- [ ] Unit tests pass (`pytest`) -- [ ] Minimal system quicktest passes (`pytest -s minimal_system_quick_test.py`) -- [ ] All integration tests pass (`daqsystemtest_integtest_bundle.sh`) - [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) -- [ ] Testing skipped as this is a documentation PR +- [ ] Unit tests pass (`pytest`) +- [ ] Minimal system quick test passes (`pytest -s minimal_system_quick_test.py`) +- [ ] Integration tests pass (`daqsystemtest_integtest_bundle.sh`) +- [ ] Drunc integration tests pass (`pytest -m integration_tests`) Note - at the time of creating this template, these tests have not been written hence remain as a TODO. +- [ ] Testing skipped as there are no core code changes in this PR -## Type of change +## Manual testing checklist + +_If applicable, include a description of what should be run to demonstrate the changes that are being made._ -- [ ] New feature or enhancement (non-breaking change which adds functionality) -- [ ] Optimization (non-breaking change that improves code/performance) -- [ ] Bug fix (non-breaking change which fixes an issue) -- [ ] Breaking change (whatever its nature) -- [ ] Documentation (non-breaking change that adds or improves the documentation) ## Further checks From 3f19a18f881d1a572a91baabfa8637b0e241354f Mon Sep 17 00:00:00 2001 From: PawelPlesniak Date: Tue, 25 Nov 2025 15:41:49 +0100 Subject: [PATCH 03/14] Clarifying details on the integration test bundle --- .github/pull_request_template.md | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 8085edcfd..90df6d30b 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -15,23 +15,21 @@ _This issue should contain the full description of what the PR is intended to de If applicable, list all the related PRs required to implement and test this change. -## Changelog +## Change log _Include a high level overview of the changes introduced by this PR._ -## Testing checklist +## Testing checklist, complete prior to marking this as "Ready for Review" Testing was ran on: _include the host name here_ - [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) - [ ] Unit tests pass (`pytest`) - [ ] Minimal system quick test passes (`pytest -s minimal_system_quick_test.py`) -- [ ] Integration tests pass (`daqsystemtest_integtest_bundle.sh`) - [ ] Drunc integration tests pass (`pytest -m integration_tests`) Note - at the time of creating this template, these tests have not been written hence remain as a TODO. - - [ ] Testing skipped as there are no core code changes in this PR -## Manual testing checklist +## Suggested manual testing checklist _If applicable, include a description of what should be run to demonstrate the changes that are being made._ @@ -39,6 +37,23 @@ _If applicable, include a description of what should be run to demonstrate the c ## Further checks - [ ] Code is commented where needed, particularly in hard-to-understand areas -- [ ] Code style is correct (`dbt-build --lint`, and/or see https://dune-daq-sw.readthedocs.io/en/latest/packages/styleguide/) - [ ] If applicable, new tests have been added or an issue has been opened to tackle that in the future. (Indicate issue here: # (issue)) + +_The option below is awaiting discussion with SWIT regarding global linting and styling, and separate templates for the different repo types_ +- [ ] Code style is correct (`dbt-build --lint`, and/or see https://dune-daq-sw.readthedocs.io/en/latest/packages/styleguide/) + +# Reviewer checklist + +- [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) +- [ ] Unit tests pass (`pytest`) +- [ ] Suggested manual tests pass +- [ ] Drunc integration tests pass (`pytest -m integration_tests`) Note - at the time of creating this template, these tests have not been written hence remain as a TODO. +- [ ] Minimal system quick test passes (`pytest -s minimal_system_quick_test.py`) +- [ ] Integration tests pass (`daqsystemtest_integtest_bundle.sh`) + - This test takes a long time, it can be left running on its own, and does not have to be monitored as it runs. + - The only check that the `drunc` developers should be concerned about in is whether any issues related to `drunc` are mentioned in any of the log files. + - If the issue raised is not related to run control, use the following steps, only move on to the next step if the previous step fails: + - Validate in a fresh working area + - Contact the lead developer + - Notify on the `#daq-release-prep channel` on Slack \ No newline at end of file From 1535e58daabcd47ea2506683abcced48ca433fa6 Mon Sep 17 00:00:00 2001 From: PawelPlesniak Date: Mon, 1 Dec 2025 12:56:13 +0100 Subject: [PATCH 04/14] Adding supplemental review block --- .github/pull_request_template.md | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 90df6d30b..196f034ed 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,6 +1,6 @@ # Description - Addresses issue # + _This issue should contain the full description of what the PR is intended to deliver._ ## Type of change @@ -12,15 +12,17 @@ _This issue should contain the full description of what the PR is intended to de - [ ] Documentation (non-breaking change that adds or improves the documentation) ## Requires - If applicable, list all the related PRs required to implement and test this change. ## Change log _Include a high level overview of the changes introduced by this PR._ -## Testing checklist, complete prior to marking this as "Ready for Review" +## Suggested manual testing checklist + +_If applicable, include a description of what should be run to demonstrate the changes that are being made._ +## Testing checklist, complete prior to marking this as "Ready for Review" Testing was ran on: _include the host name here_ - [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) @@ -29,10 +31,6 @@ Testing was ran on: _include the host name here_ - [ ] Drunc integration tests pass (`pytest -m integration_tests`) Note - at the time of creating this template, these tests have not been written hence remain as a TODO. - [ ] Testing skipped as there are no core code changes in this PR -## Suggested manual testing checklist - -_If applicable, include a description of what should be run to demonstrate the changes that are being made._ - ## Further checks @@ -41,9 +39,10 @@ _If applicable, include a description of what should be run to demonstrate the c (Indicate issue here: # (issue)) _The option below is awaiting discussion with SWIT regarding global linting and styling, and separate templates for the different repo types_ -- [ ] Code style is correct (`dbt-build --lint`, and/or see https://dune-daq-sw.readthedocs.io/en/latest/packages/styleguide/) +- [ ] Code style is correct (`dbt-build --lint`, see [the documentation](https://dune-daq-sw.readthedocs.io/en/latest/packages/styleguide/)) # Reviewer checklist +_Note - if a reveiwer requests changes and those changes are implemented, it is expected that the below block is re-ran._ - [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) - [ ] Unit tests pass (`pytest`) From aee4436a68040d178670c08f790a9b8da4f44a67 Mon Sep 17 00:00:00 2001 From: PawelPlesniak Date: Thu, 5 Feb 2026 13:47:08 +0100 Subject: [PATCH 05/14] Update prior to review with Kurt --- .github/pull_request_template.md | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 196f034ed..829b074f8 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -12,24 +12,20 @@ _This issue should contain the full description of what the PR is intended to de - [ ] Documentation (non-breaking change that adds or improves the documentation) ## Requires -If applicable, list all the related PRs required to implement and test this change. +_If applicable, list all the related PRs required to implement and test this change._ ## Change log _Include a high level overview of the changes introduced by this PR._ -## Suggested manual testing checklist - -_If applicable, include a description of what should be run to demonstrate the changes that are being made._ - -## Testing checklist, complete prior to marking this as "Ready for Review" -Testing was ran on: _include the host name here_ +## Developer testing checklist, complete prior to marking this as "Ready for Review" +Tests ran on: _include the host name here_ - [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) - [ ] Unit tests pass (`pytest`) -- [ ] Minimal system quick test passes (`pytest -s minimal_system_quick_test.py`) -- [ ] Drunc integration tests pass (`pytest -m integration_tests`) Note - at the time of creating this template, these tests have not been written hence remain as a TODO. -- [ ] Testing skipped as there are no core code changes in this PR +- [ ] Unit tests ran, or at least minimal system quick test run successfully (`pytest -s minimal_system_quick_test.py`) +- [ ] Testing skipped as there are no core code changes in this PR, this only relates to documentation/CI workflows + ## Further checks @@ -41,8 +37,19 @@ Testing was ran on: _include the host name here_ _The option below is awaiting discussion with SWIT regarding global linting and styling, and separate templates for the different repo types_ - [ ] Code style is correct (`dbt-build --lint`, see [the documentation](https://dune-daq-sw.readthedocs.io/en/latest/packages/styleguide/)) +## Developer notes for the reviewer + +- [ ] I have run the minimal system quick test only, please run the rest of the tests +- [ ] I have run the full integration tests bundle, this can be skipped + +_If applicable, leave any other guidance here._ + +## Suggested manual testing checklist + +_If applicable, include a description of what should be run to demonstrate and test the changes that are being made._ + # Reviewer checklist -_Note - if a reveiwer requests changes and those changes are implemented, it is expected that the below block is re-ran._ +_Note - if a reveiwer requests changes and those changes are implemented, this block should be re-checked._ - [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) - [ ] Unit tests pass (`pytest`) From 8db537488d0b6a6438c0839b6c81cd14e1bcfb44 Mon Sep 17 00:00:00 2001 From: PawelPlesniak Date: Thu, 5 Feb 2026 17:00:09 +0100 Subject: [PATCH 06/14] Testing checklist finalized --- .github/pull_request_template.md | 34 ++++++++++++++------------------ 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 829b074f8..df550a2f3 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,8 +1,10 @@ # Description -Addresses issue # +Addresses issue # _Fill this in with the relevant issue number so the relevant issue can be closed._ _This issue should contain the full description of what the PR is intended to deliver._ +_Start with a brief description of what this PR changes._ + ## Type of change - [ ] New feature or enhancement (non-breaking change which adds functionality) @@ -11,7 +13,7 @@ _This issue should contain the full description of what the PR is intended to de - [ ] Breaking change (whatever its nature) - [ ] Documentation (non-breaking change that adds or improves the documentation) -## Requires +## List of required branches from other repositories _If applicable, list all the related PRs required to implement and test this change._ ## Change log @@ -22,8 +24,8 @@ _Include a high level overview of the changes introduced by this PR._ Tests ran on: _include the host name here_ - [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) -- [ ] Unit tests pass (`pytest`) -- [ ] Unit tests ran, or at least minimal system quick test run successfully (`pytest -s minimal_system_quick_test.py`) +- [ ] Unit tests pass (`pytest`) - note please use the broadest marker possible +- [ ] Integration tests ran (`daqsystemtest_integtest_bundle.sh`) successfully, or at least minimal system quick test run successfully (`pytest -s minimal_system_quick_test.py`) - [ ] Testing skipped as there are no core code changes in this PR, this only relates to documentation/CI workflows @@ -31,19 +33,14 @@ Tests ran on: _include the host name here_ ## Further checks - [ ] Code is commented where needed, particularly in hard-to-understand areas -- [ ] If applicable, new tests have been added or an issue has been opened to tackle that in the future. - (Indicate issue here: # (issue)) - -_The option below is awaiting discussion with SWIT regarding global linting and styling, and separate templates for the different repo types_ -- [ ] Code style is correct (`dbt-build --lint`, see [the documentation](https://dune-daq-sw.readthedocs.io/en/latest/packages/styleguide/)) +- [ ] If applicable, new tests have been added or an issue has been opened to tackle that in the future in : # -## Developer notes for the reviewer +# Developer notes for the reviewer +## Integration testing completed by the developer - [ ] I have run the minimal system quick test only, please run the rest of the tests - [ ] I have run the full integration tests bundle, this can be skipped -_If applicable, leave any other guidance here._ - ## Suggested manual testing checklist _If applicable, include a description of what should be run to demonstrate and test the changes that are being made._ @@ -52,14 +49,13 @@ _If applicable, include a description of what should be run to demonstrate and t _Note - if a reveiwer requests changes and those changes are implemented, this block should be re-checked._ - [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) -- [ ] Unit tests pass (`pytest`) -- [ ] Suggested manual tests pass -- [ ] Drunc integration tests pass (`pytest -m integration_tests`) Note - at the time of creating this template, these tests have not been written hence remain as a TODO. -- [ ] Minimal system quick test passes (`pytest -s minimal_system_quick_test.py`) +- [ ] Unit tests pass (`pytest`) - note please use the broadest marker possible +- [ ] Suggested manual tests pass as described above - [ ] Integration tests pass (`daqsystemtest_integtest_bundle.sh`) - This test takes a long time, it can be left running on its own, and does not have to be monitored as it runs. - - The only check that the `drunc` developers should be concerned about in is whether any issues related to `drunc` are mentioned in any of the log files. + - The only check that the `drunc` developers should be concerned about in is whether any issues related to `drunc` are mentioned in any of the log files - If the issue raised is not related to run control, use the following steps, only move on to the next step if the previous step fails: - Validate in a fresh working area - - Contact the lead developer - - Notify on the `#daq-release-prep channel` on Slack \ No newline at end of file + - Contact the lead developer of the relevant WG (contact Run Control lead if unsure) + - Notify on the `#daq-release-prep channel` on Slack once another developer has verified this issue + \ No newline at end of file From 367f86976b6573fb3eeb626c20d8591f74e6891c Mon Sep 17 00:00:00 2001 From: PawelPlesniak Date: Thu, 5 Feb 2026 17:01:20 +0100 Subject: [PATCH 07/14] Note on integtests --- .github/pull_request_template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index df550a2f3..3ef7d5621 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -25,7 +25,7 @@ Tests ran on: _include the host name here_ - [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) - [ ] Unit tests pass (`pytest`) - note please use the broadest marker possible -- [ ] Integration tests ran (`daqsystemtest_integtest_bundle.sh`) successfully, or at least minimal system quick test run successfully (`pytest -s minimal_system_quick_test.py`) +- [ ] Integration tests ran (`daqsystemtest_integtest_bundle.sh`) successfully, or at least minimal system quick test run successfully (`pytest -s minimal_system_quick_test.py`). Note - these are resource intensive and it is recommended that these are not run locally - [ ] Testing skipped as there are no core code changes in this PR, this only relates to documentation/CI workflows From 1dfaf6c545f32a0fe7b46164214360a18a72fcc7 Mon Sep 17 00:00:00 2001 From: PawelPlesniak Date: Thu, 5 Feb 2026 17:04:41 +0100 Subject: [PATCH 08/14] testing list done --- .github/pull_request_template.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 3ef7d5621..2b1a97fa5 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -20,7 +20,7 @@ _If applicable, list all the related PRs required to implement and test this cha _Include a high level overview of the changes introduced by this PR._ -## Developer testing checklist, complete prior to marking this as "Ready for Review" +## Initial checklist prior to marking this as "Ready for Review" Tests ran on: _include the host name here_ - [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) @@ -29,11 +29,10 @@ Tests ran on: _include the host name here_ - [ ] Testing skipped as there are no core code changes in this PR, this only relates to documentation/CI workflows - -## Further checks +## Final checklist prior to marking this as "Ready for Review" - [ ] Code is commented where needed, particularly in hard-to-understand areas -- [ ] If applicable, new tests have been added or an issue has been opened to tackle that in the future in : # +- [ ] New unit tests have been added or an issue has been opened to tackle that in the future in : # # Developer notes for the reviewer From 7fd2217dcc3f08c8a754c9b3966c50fc30747e83 Mon Sep 17 00:00:00 2001 From: PawelPlesniak Date: Thu, 5 Feb 2026 17:10:13 +0100 Subject: [PATCH 09/14] Final notes --- .github/pull_request_template.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 2b1a97fa5..ac8843b8f 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -35,6 +35,7 @@ Tests ran on: _include the host name here_ - [ ] New unit tests have been added or an issue has been opened to tackle that in the future in : # # Developer notes for the reviewer +Please select a reviewer from the group this PR relates the most to from list of primary developers [listed in the wiki](https://github.com/DUNE-DAQ/drunc/wiki#active-developers). ## Integration testing completed by the developer - [ ] I have run the minimal system quick test only, please run the rest of the tests From 0c7dad5c85156a059431d15cd5b6f6bdb6e70294 Mon Sep 17 00:00:00 2001 From: PawelPlesniak Date: Thu, 5 Feb 2026 17:10:27 +0100 Subject: [PATCH 10/14] Should have git added again --- .github/pull_request_template.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index ac8843b8f..cd5fda830 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -58,4 +58,6 @@ _Note - if a reveiwer requests changes and those changes are implemented, this b - Validate in a fresh working area - Contact the lead developer of the relevant WG (contact Run Control lead if unsure) - Notify on the `#daq-release-prep channel` on Slack once another developer has verified this issue - \ No newline at end of file + + +Once the features are validated and both the unit and integrationm tests pass, the PRs can be merged. \ No newline at end of file From 417e68f131f79896373aef8965ca9c5afefc8061 Mon Sep 17 00:00:00 2001 From: PawelPlesniak Date: Fri, 6 Feb 2026 16:21:16 +0100 Subject: [PATCH 11/14] Updated --- .github/pull_request_template.md | 84 +++++++++++++++++++++++++------- 1 file changed, 66 insertions(+), 18 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index cd5fda830..493c458d3 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,10 +1,14 @@ # Description -Addresses issue # _Fill this in with the relevant issue number so the relevant issue can be closed._ - -_This issue should contain the full description of what the PR is intended to deliver._ +Fixes issue # _Fill this in with the relevant issue number so the relevant issue can be closed._ _Start with a brief description of what this PR changes._ +_If the user workflow has changed, otherwise delete the following line_ + +The relevant changes in the user workflow have been documented _here_ (link URL) + + + ## Type of change - [ ] New feature or enhancement (non-breaking change which adds functionality) @@ -23,41 +27,85 @@ _Include a high level overview of the changes introduced by this PR._ ## Initial checklist prior to marking this as "Ready for Review" Tests ran on: _include the host name here_ -- [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) -- [ ] Unit tests pass (`pytest`) - note please use the broadest marker possible -- [ ] Integration tests ran (`daqsystemtest_integtest_bundle.sh`) successfully, or at least minimal system quick test run successfully (`pytest -s minimal_system_quick_test.py`). Note - these are resource intensive and it is recommended that these are not run locally +Unit tests - for some stages of development, additional deeper tests have been written to test functionality that cannot be ran as part of a CI. These tests have been separated out with [run control specific pytest markers](https://github.com/DUNE-DAQ/drunc/wiki/Testing-prior-to-PR-merges). If the development in this PR is relevant to the scope of these pytest markers, run them as is documented in the link. Otherwise the CI workflow will run the base (unmarked) tests. + +Integration tests - the `daqsystemtest_integtest_bundle` will need to be run on a server with large resources and connections to the EHN1 infrastructure. Check the [cross referenced list](https://github.com/DUNE-DAQ/drunc/wiki#users-with-access-to-clusters-for-running-daqsystemtest_integtest_bundlesh) of developers who have access to clusters where the full testing bundle can be ran. The developer needs to run at least the + +Note - Most institutional clusters should be able to run these tests. See the [notes on running when away from CERN or FNAL](https://github.com/DUNE-DAQ/drunc/wiki/Testing-when-not-at-cern-fnal). + +- Unit tests (`pytest --marker`) passed + - [ ] Ran with relevant marker + - [ ] Skipped as no marker relevant +- Integration tests (`daqsystemtest_integtest_bundle.sh`) passed + - [ ] Ran `daqsystemtest_integtest_bundle.sh -k minimal_system_quick_test.py` only + - [ ] Ran `daqsystemtest_integtest_bundle.sh` fully - [ ] Testing skipped as there are no core code changes in this PR, this only relates to documentation/CI workflows ## Final checklist prior to marking this as "Ready for Review" -- [ ] Code is commented where needed, particularly in hard-to-understand areas +- [ ] Code changes are commented where needed, particularly in hard-to-understand areas - [ ] New unit tests have been added or an issue has been opened to tackle that in the future in : # # Developer notes for the reviewer -Please select a reviewer from the group this PR relates the most to from list of primary developers [listed in the wiki](https://github.com/DUNE-DAQ/drunc/wiki#active-developers). - -## Integration testing completed by the developer -- [ ] I have run the minimal system quick test only, please run the rest of the tests -- [ ] I have run the full integration tests bundle, this can be skipped +Please select and assign a reviewer from the group this PR relates the most to from list of primary developers [listed in the wiki](https://github.com/DUNE-DAQ/drunc/wiki#active-developers). ## Suggested manual testing checklist -_If applicable, include a description of what should be run to demonstrate and test the changes that are being made._ +_If applicable, include a description of what should be executed to demonstrate and test the features, bug fixes, or other changes made within this PR._ # Reviewer checklist _Note - if a reveiwer requests changes and those changes are implemented, this block should be re-checked._ -- [ ] Pre-commit hooks run successfully if applicable (e.g. `pre-commit run --all-files`) -- [ ] Unit tests pass (`pytest`) - note please use the broadest marker possible - [ ] Suggested manual tests pass as described above -- [ ] Integration tests pass (`daqsystemtest_integtest_bundle.sh`) +- [ ] All CI workflows passed/failures documented +- [ ] Integration tests passed (`daqsystemtest_integtest_bundle.sh. --stop-on-failure`) - This test takes a long time, it can be left running on its own, and does not have to be monitored as it runs. - The only check that the `drunc` developers should be concerned about in is whether any issues related to `drunc` are mentioned in any of the log files - If the issue raised is not related to run control, use the following steps, only move on to the next step if the previous step fails: - Validate in a fresh working area - Contact the lead developer of the relevant WG (contact Run Control lead if unsure) - Notify on the `#daq-release-prep channel` on Slack once another developer has verified this issue - + + +Once the features are validated and both the unit and integration tests pass, the PRs is ready to be merged. + +# Prior to merging +## For documentation only PRs + +- [ ] All CI workflows have passed + +## For all other PR types +Choose one of the following an complete all substeps + +- These changes are internal to the Run Control and in a single repository, and do not affect the end user. + - [ ] The relevant changes have been thoroughly documented in docstrings and code comments + - [ ] The wiki has been updated if it is relevant to do so (architectural or endpoint changes). +- The changes affect the workflow of end users, or span multiple repositories + - [ ] Workflow changes have been demonstrated in the Change Log + - [ ] The relevant sections have been documented here in the wiki (link is in the Description section) + - [ ] A message has been posted to the #daq-sw-librarians Slack channel in the DUNE workspace to announce this to the broader developer community. + +Once the requested reviewer is satisfied with the PR, the PR can be merged. + +## Notification message for #daq-sw-librarians Slack channel + +### For an single merge that changes the user workflow +``` +The CCM WG has an isolated PR ready to merge that affects user workflows. The PR is: + +_URL_ + +I will leave time for any comments, otherwise will merge these at the end of the work day _Insert your time zone_. +``` +### For co-ordinated merge +``` +The CCM WG has a set of co-ordinated merges ready to merge. The PRs are: + +_URL_ + +_URL_ + -Once the features are validated and both the unit and integrationm tests pass, the PRs can be merged. \ No newline at end of file +I will leave time for any comments, otherwise will merge these at the end of the day. +``` \ No newline at end of file From 9b6d304eec8142c62b1de8545caf39a77ccb08e6 Mon Sep 17 00:00:00 2001 From: PawelPlesniak Date: Fri, 27 Feb 2026 18:57:28 +0100 Subject: [PATCH 12/14] Review --- .github/pull_request_template.md | 120 ++++++++++++++++++------------- 1 file changed, 69 insertions(+), 51 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 493c458d3..96da0de62 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,9 +1,9 @@ # Description -Fixes issue # _Fill this in with the relevant issue number so the relevant issue can be closed._ +Fixes issue # _ISSUE NUMBER_ -_Start with a brief description of what this PR changes._ +_WHAT DOES THIS PR CHANGE - ONE LINE._ -_If the user workflow has changed, otherwise delete the following line_ +_DOCUMENT THE CHANGE BELOW OR DELETE IT_ The relevant changes in the user workflow have been documented _here_ (link URL) @@ -11,84 +11,99 @@ The relevant changes in the user workflow have been documented _here_ (link URL) ## Type of change -- [ ] New feature or enhancement (non-breaking change which adds functionality) -- [ ] Optimization (non-breaking change that improves code/performance) -- [ ] Bug fix (non-breaking change which fixes an issue) -- [ ] Breaking change (whatever its nature) -- [ ] Documentation (non-breaking change that adds or improves the documentation) +- [ ] New feature / enhancement +- [ ] Optimization +- [ ] Bug fix +- [ ] Breaking change +- [ ] Documentation ## List of required branches from other repositories -_If applicable, list all the related PRs required to implement and test this change._ +_WHAT PRs NEED TO BE INCLUDED TO MAKE THE CHANGE._ ## Change log -_Include a high level overview of the changes introduced by this PR._ +_WHAT HAS CHANGED._ -## Initial checklist prior to marking this as "Ready for Review" -Tests ran on: _include the host name here_ +## Suggested manual testing checklist + +_LIST COMMANDS TO DEMONSTRATE CHANGE_ + +
+ + +# Developer checklist -Unit tests - for some stages of development, additional deeper tests have been written to test functionality that cannot be ran as part of a CI. These tests have been separated out with [run control specific pytest markers](https://github.com/DUNE-DAQ/drunc/wiki/Testing-prior-to-PR-merges). If the development in this PR is relevant to the scope of these pytest markers, run them as is documented in the link. Otherwise the CI workflow will run the base (unmarked) tests. + +## Prior to marking this as "Ready for Review" -Integration tests - the `daqsystemtest_integtest_bundle` will need to be run on a server with large resources and connections to the EHN1 infrastructure. Check the [cross referenced list](https://github.com/DUNE-DAQ/drunc/wiki#users-with-access-to-clusters-for-running-daqsystemtest_integtest_bundlesh) of developers who have access to clusters where the full testing bundle can be ran. The developer needs to run at least the +Tests ran on: _WHAT HOSTNAME_ -Note - Most institutional clusters should be able to run these tests. See the [notes on running when away from CERN or FNAL](https://github.com/DUNE-DAQ/drunc/wiki/Testing-when-not-at-cern-fnal). +Unit tests - some tests can't be ran on the CI. This is [documented](https://github.com/DUNE-DAQ/drunc/wiki/Testing-prior-to-PR-merges). If this PR checks a feature that can't be tested with CI, this has been marked appropriately. + +Integration tests - the `daqsystemtest_integtest_bundle` requires a lot of resources, and connections to the EHN1 infrastructure. Check the [cross referenced list](https://github.com/DUNE-DAQ/drunc/wiki#users-with-access-to-clusters-for-running-daqsystemtest_integtest_bundlesh) if you can't run these. The developer needs to run at least the [.](https://github.com/DUNE-DAQ/daqsystemtest/blob/develop/integtest/minimal_system_quick_test.py) - Unit tests (`pytest --marker`) passed - - [ ] Ran with relevant marker - - [ ] Skipped as no marker relevant -- Integration tests (`daqsystemtest_integtest_bundle.sh`) passed - - [ ] Ran `daqsystemtest_integtest_bundle.sh -k minimal_system_quick_test.py` only - - [ ] Ran `daqsystemtest_integtest_bundle.sh` fully + - [ ] With relevant marker + - [ ] Without marker +- Integration tests passed + - [ ] Only `daqsystemtest_integtest_bundle.sh -k minimal_system_quick_test.py` + - [ ] Full `daqsystemtest_integtest_bundle.sh` - [ ] Testing skipped as there are no core code changes in this PR, this only relates to documentation/CI workflows ## Final checklist prior to marking this as "Ready for Review" -- [ ] Code changes are commented where needed, particularly in hard-to-understand areas -- [ ] New unit tests have been added or an issue has been opened to tackle that in the future in : # - -# Developer notes for the reviewer -Please select and assign a reviewer from the group this PR relates the most to from list of primary developers [listed in the wiki](https://github.com/DUNE-DAQ/drunc/wiki#active-developers). +- [ ] Code is clearly commented. +- [ ] New unit tests have been added, or is documented in # _ISSUE NUMBER_ +- [ ] A suitable reviewer has been chosen from [this list](https://github.com/DUNE-DAQ/drunc/wiki#active-developers). -## Suggested manual testing checklist +
-_If applicable, include a description of what should be executed to demonstrate and test the features, bug fixes, or other changes made within this PR._ +
+ # Reviewer checklist -_Note - if a reveiwer requests changes and those changes are implemented, this block should be re-checked._ - -- [ ] Suggested manual tests pass as described above -- [ ] All CI workflows passed/failures documented -- [ ] Integration tests passed (`daqsystemtest_integtest_bundle.sh. --stop-on-failure`) - - This test takes a long time, it can be left running on its own, and does not have to be monitored as it runs. - - The only check that the `drunc` developers should be concerned about in is whether any issues related to `drunc` are mentioned in any of the log files - - If the issue raised is not related to run control, use the following steps, only move on to the next step if the previous step fails: - - Validate in a fresh working area - - Contact the lead developer of the relevant WG (contact Run Control lead if unsure) - - Notify on the `#daq-release-prep channel` on Slack once another developer has verified this issue + + + +- [ ] This branch has been rebased with develop prior to testing. +- [ ] Suggested manual tests show changes. +- [ ] CI workflows fails documented (if present) +- [ ] Integration tests passed + - Only concern yourself if failures related to `drunc` are in the log files + - If non-`drunc` failure appears: + - Validate failure in fresh working area + - Contact Pawel if unsure Once the features are validated and both the unit and integration tests pass, the PRs is ready to be merged. -# Prior to merging -## For documentation only PRs +
-- [ ] All CI workflows have passed +
+ -## For all other PR types +# Prior to merging + +
Choose one of the following an complete all substeps -- These changes are internal to the Run Control and in a single repository, and do not affect the end user. - - [ ] The relevant changes have been thoroughly documented in docstrings and code comments - - [ ] The wiki has been updated if it is relevant to do so (architectural or endpoint changes). -- The changes affect the workflow of end users, or span multiple repositories - - [ ] Workflow changes have been demonstrated in the Change Log - - [ ] The relevant sections have been documented here in the wiki (link is in the Description section) - - [ ] A message has been posted to the #daq-sw-librarians Slack channel in the DUNE workspace to announce this to the broader developer community. +- Changes only affect the Run Control, are in a single repository, and do not affect the end user. + - [ ] Changes are documented in docstrings and code comments + - [ ] Wiki has been updated if architectural or endpoint changes +- Otherwise + - [ ] Workflow changes demonstrated in the Change Log (if necessary) + - [ ] Wiki has been updated (if necessary) + - [ ] #daq-sw-librarians Slack channel notified (see below) -Once the requested reviewer is satisfied with the PR, the PR can be merged. +Once completed, the reviewer can merge the PR. +
+ + ## Notification message for #daq-sw-librarians Slack channel + +
### For an single merge that changes the user workflow ``` @@ -108,4 +123,7 @@ _URL_ I will leave time for any comments, otherwise will merge these at the end of the day. -``` \ No newline at end of file +``` + +
+
\ No newline at end of file From 6a2eac29d7449a8dbb44b978395f850d63fa0df2 Mon Sep 17 00:00:00 2001 From: PawelPlesniak Date: Fri, 27 Feb 2026 19:00:02 +0100 Subject: [PATCH 13/14] Adding nightly request requirement --- .github/pull_request_template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 96da0de62..7ed1b45e0 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -36,7 +36,7 @@ _LIST COMMANDS TO DEMONSTRATE CHANGE_ ## Prior to marking this as "Ready for Review" -Tests ran on: _WHAT HOSTNAME_ +Tests ran on: _WHAT HOSTNAME_ from release _RELEASE_NAME_ Unit tests - some tests can't be ran on the CI. This is [documented](https://github.com/DUNE-DAQ/drunc/wiki/Testing-prior-to-PR-merges). If this PR checks a feature that can't be tested with CI, this has been marked appropriately. From 681d26bba72011c3867418ea6fcd8d36c3455b81 Mon Sep 17 00:00:00 2001 From: Emir Muhammad Date: Mon, 2 Mar 2026 16:26:54 +0100 Subject: [PATCH 14/14] Minor formatting fixes --- .github/pull_request_template.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 7ed1b45e0..aa08153ee 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -34,6 +34,7 @@ _LIST COMMANDS TO DEMONSTRATE CHANGE_ # Developer checklist + ## Prior to marking this as "Ready for Review" Tests ran on: _WHAT HOSTNAME_ from release _RELEASE_NAME_ @@ -85,7 +86,6 @@ Once the features are validated and both the unit and integration tests pass, th # Prior to merging -
Choose one of the following an complete all substeps - Changes only affect the Run Control, are in a single repository, and do not affect the end user. @@ -97,13 +97,12 @@ Choose one of the following an complete all substeps - [ ] #daq-sw-librarians Slack channel notified (see below) Once completed, the reviewer can merge the PR. -
+
## Notification message for #daq-sw-librarians Slack channel -
### For an single merge that changes the user workflow ```