Skip to content
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.

Sign up for GitHub

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

Jump to bottom

pull_request_template: clearify the template and remove checkbox verification #18708

Closed
wants to merge 1 commit into from
Closed

pull_request_template: clearify the template and remove checkbox verification #18708

wants to merge 1 commit into from

Conversation

yaronkaikov
Copy link
Contributor

@yaronkaikov yaronkaikov commented May 16, 2024 •
edited

It seems that having the checkbox in the PR template and failing the action is confusing and not very clear. Let's remove it completely and just add to the template an explanation to explain the backport reason

Please explain below if this patch should be backported or not

Fixing template for PR, not need for backport

@yaronkaikov yaronkaikov force-pushed the improve-pr-template branch 3 times, most recently from 9c0cd3a to f991c82 Compare May 16, 2024 13:32
@yaronkaikov yaronkaikov added the backport/none Backport is not required label May 16, 2024
@yaronkaikov yaronkaikov mentioned this pull request May 16, 2024
Closed
@yaronkaikov yaronkaikov requested a review from nyh May 16, 2024 13:33
@nyh nyh requested a review from avikivity May 16, 2024 13:50
@nyh
Copy link
Contributor

nyh commented May 16, 2024

Please explain below if this patch should be backported or not

Maybe this should be even clearer? The word "below" suggests maybe you are supposed keep this line, and add something below. I don't think that's what we want. So maybe something like (I just wrote something quickly, the wording can probably be improved)

Please replace this line by justification for the backport/* labels put on for this PR

@yaronkaikov
Copy link
Contributor Author

Please explain below if this patch should be backported or not

Maybe this should be even clearer? The word "below" suggests maybe you are supposed keep this line, and add something below. I don't think that's what we want. So maybe something like (I just wrote something quickly, the wording can probably be improved)

Please replace this line by justification for the backport/* labels put on for this PR

Done

nyh
nyh reviewed May 16, 2024
View reviewed changes
.github/pull_request_template.md Outdated
@@ -1,2 +1 @@
- [ ] ** Backport reason (please explain below if this patch should be backported or not) **

**Please replace this line by justification for the backport/\* labels put on for this PR**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the typo in my original suggestion, but "on for" was a typo - should probably be just "on".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed a bit, I think it makes more sense ,

@yaronkaikov
pull_request_template: clearify the template and remove checkbox veri…
58666f6 
…fication

It seems that having the checkbox in the PR template and failing the action is confusing and not very clear. Let's remove it completely and just add to the template an explanation to explain the backport reason
@yaronkaikov yaronkaikov requested a review from nyh May 16, 2024 18:06
@yaronkaikov
Copy link
Contributor Author

@nyh please review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nyh nyh nyh approved these changes

@avikivity avikivity Awaiting requested review from avikivity

Assignees
No one assigned
Labels
backport/none Backport is not required promoted-to-master
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@yaronkaikov @nyh