Skip to content

chore: PR template and validation updates #3577

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

Closed
wants to merge 14 commits into from

Conversation

michalChrobot
Copy link
Collaborator

@michalChrobot michalChrobot commented Aug 4, 2025

Purpose of this PR

This PR aims to address some areas for the ease of Playtesting and validation of our changes. Important to note are the following changes:

Updated PR template with more sections where (REQUIRED for a PR to be merged) Documentation, Testing and QA, and Backport sections. I renamed the git workflow and I will update the name in the settings when merging this PR so for now you can see backport-verification pending since it doesn't exist anymore.
Newly introduced sections are

  • Testing and Documentation section was divided into separate Documentation and Testing and QA sections
  • Documentation section is REQUIRED for the PR to be merged and explains if/what documentation changes/corrections were made to accompany code changes
  • Testing and QA section is REQUIRED for the PR to be merged and explains what testing coverage was added for those changes
  • Jira ticket section was added for consideration if such PR is big enough to require Jira ticket present. Note that this section is not mandatory but may be flagged by QA when evaluating PRs for release

Such updated PR template will look as follows
image

Another thing to note is that Documentation section was added to the changelog to group any changes of this kind (since it would be unintuitive to count them as fixed or added)

One more thing that I'm updating is that I failed to notice that targets and drafts conditions are configured as separate items in a list. In our CI, basic triggers in a list are combined with an OR operator, meaning the job will run if any of the conditions are met. This lead to test being executed on draft PRs. What I'm changing is

  1. The tests will not be required for PRs touching only the package documentation
  2. The above change wouldn't be possible without introducing comment triggers. For example the check would still require this job on PRs but it wouldn't be triggered due to documentation only changes. Because of this I added a new check in a same way as in N4E which requires you to type comment of either /ci ngo OR /ci ignore. The first one will probably trigger the PR checks (but note that if your PR contains only documentation changes then it will just pass without running it), the second one will indicate that YOU TAKE RESPONSIBILITY AND YOU ACKNOWLEDGE that not testing is needed for your PR

Note that the test will execute now on whatever branch if we will specifically want that (by typing /ci ngo)

Jira ticket

MTT-12822

Documentation

No documentation changes or additions were necessary.

Testing & QA

This change won't affect users and focuses around our workflow and templates so no testing is needed

Backport

Will be backported when we agree to the shape of it

@michalChrobot michalChrobot self-assigned this Aug 4, 2025
@michalChrobot michalChrobot marked this pull request as ready for review August 4, 2025 12:58
@michalChrobot michalChrobot changed the title chore: PR template updates chore: PR template and validation updates Aug 4, 2025
@michalChrobot
Copy link
Collaborator Author

Note that currently backport-verification and Pull Request Trigger are still required as those are taken from repo settings and I need to modify those (which will affect all other PRs) to make 100% sure that my changes work as expected.

When we will agree to merge this PR then I will

  1. Create a backport to develop branch
  2. Modify the repo settings
  3. See if it works correctly for this PR and merge it.
  4. Create a test PR that touches only Documentation to verify the workflow

So just notice that it may take me up to 20m of possible disruption. After that ALL PRs will need to pull latest changes so they work correctly

@michalChrobot
Copy link
Collaborator Author

I decided to split this PR into 2 independent ones, you can follow up on PR template in #3582 while the PR comment triggers and CI optimization is in #3580

@michalChrobot michalChrobot deleted the pr-template branch August 5, 2025 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant