You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/04/01 00:26:28 UTC
[GitHub] [pinot] sajjad-moradi opened a new pull request #8455: Make the PR template simpler
sajjad-moradi opened a new pull request #8455:
URL: https://github.com/apache/pinot/pull/8455
Nowadays people usually don't follow the current PR template. They either delete the whole thing and add the description text, or they add the description on top of the prefilled template.
This PR makes the template simpler and the only ask from author is to add proper labels. This will be super helpful during the release phase. I had to go over 300 commits one by one and let's say the experience wasn't pleasant ;)
I added all PMC members as reviewers for this PR to get a consensus on it and afterwards committers as well as reviewers should follow the new simple process.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] KKcorps commented on a change in pull request #8455: Make the PR template simpler
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8455:
URL: https://github.com/apache/pinot/pull/8455#discussion_r840317074
##########
File path: .github/PULL_REQUEST_TEMPLATE.md
##########
@@ -1,30 +1,15 @@
-## Description
-<!-- Add a description of your PR here.
-A good description should include pointers to an issue or design document, etc.
--->
-## Upgrade Notes
-Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
-* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR fix a zero-downtime upgrade introduced earlier?
-* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR otherwise need attention when creating release notes? Things to consider:
+Instructions:
+1. The PR has to be tagged with at least one of the following labels:
+ 1. `feature`
+ 2. `bugfix`
+ 3. `performance`
+ 4. `ui`
+ 5. `backward-incompat`
+ 6. `release-notes` (*)
+2. Remove these instructions before publishing the PR.
+
+(*) Use `release-notes` label for scenarios like:
Review comment:
Another thing that should be mentioned is that tests are compulsory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on a change in pull request #8455: Make the PR template simpler
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #8455:
URL: https://github.com/apache/pinot/pull/8455#discussion_r840886771
##########
File path: .github/PULL_REQUEST_TEMPLATE.md
##########
@@ -1,30 +1,15 @@
-## Description
-<!-- Add a description of your PR here.
-A good description should include pointers to an issue or design document, etc.
--->
-## Upgrade Notes
-Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
-* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR fix a zero-downtime upgrade introduced earlier?
-* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR otherwise need attention when creating release notes? Things to consider:
+Instructions:
+1. The PR has to be tagged with at least one of the following labels:
+ 1. `feature`
+ 2. `bugfix`
+ 3. `performance`
+ 4. `ui`
+ 5. `backward-incompat`
+ 6. `release-notes` (*)
Review comment:
I would say add a lot of text in "Contribution guidelines" section in the docs, and put a link to that here?
We decide, of course :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] KKcorps commented on a change in pull request #8455: Make the PR template simpler
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8455:
URL: https://github.com/apache/pinot/pull/8455#discussion_r840315294
##########
File path: .github/PULL_REQUEST_TEMPLATE.md
##########
@@ -1,30 +1,15 @@
-## Description
-<!-- Add a description of your PR here.
-A good description should include pointers to an issue or design document, etc.
--->
-## Upgrade Notes
-Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
-* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR fix a zero-downtime upgrade introduced earlier?
-* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR otherwise need attention when creating release notes? Things to consider:
+Instructions:
+1. The PR has to be tagged with at least one of the following labels:
+ 1. `feature`
+ 2. `bugfix`
+ 3. `performance`
+ 4. `ui`
+ 5. `backward-incompat`
+ 6. `release-notes` (*)
+2. Remove these instructions before publishing the PR.
+
+(*) Use `release-notes` label for scenarios like:
Review comment:
Can we add description for all the labels mentioned in step 1. Most of the labels are self-explanatory but we can avoid their abuse by limiting the scope for each.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8455: Make the PR template simpler
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8455:
URL: https://github.com/apache/pinot/pull/8455#discussion_r840352031
##########
File path: .github/PULL_REQUEST_TEMPLATE.md
##########
@@ -1,30 +1,15 @@
-## Description
-<!-- Add a description of your PR here.
-A good description should include pointers to an issue or design document, etc.
--->
-## Upgrade Notes
-Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
-* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR fix a zero-downtime upgrade introduced earlier?
-* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR otherwise need attention when creating release notes? Things to consider:
+Instructions:
+1. The PR has to be tagged with at least one of the following labels:
+ 1. `feature`
+ 2. `bugfix`
+ 3. `performance`
+ 4. `ui`
+ 5. `backward-incompat`
+ 6. `release-notes` (*)
Review comment:
Add labels:
testing
dependencies
docker
kubernetes
observability
security
code-style
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8455: Make the PR template simpler
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8455:
URL: https://github.com/apache/pinot/pull/8455#discussion_r840352031
##########
File path: .github/PULL_REQUEST_TEMPLATE.md
##########
@@ -1,30 +1,15 @@
-## Description
-<!-- Add a description of your PR here.
-A good description should include pointers to an issue or design document, etc.
--->
-## Upgrade Notes
-Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
-* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR fix a zero-downtime upgrade introduced earlier?
-* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR otherwise need attention when creating release notes? Things to consider:
+Instructions:
+1. The PR has to be tagged with at least one of the following labels:
+ 1. `feature`
+ 2. `bugfix`
+ 3. `performance`
+ 4. `ui`
+ 5. `backward-incompat`
+ 6. `release-notes` (*)
Review comment:
Add labels:
Testing
Dependencies
Deployment
Observability
Security
Code style
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8455: Make the PR template simpler
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8455:
URL: https://github.com/apache/pinot/pull/8455#discussion_r840352031
##########
File path: .github/PULL_REQUEST_TEMPLATE.md
##########
@@ -1,30 +1,15 @@
-## Description
-<!-- Add a description of your PR here.
-A good description should include pointers to an issue or design document, etc.
--->
-## Upgrade Notes
-Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
-* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR fix a zero-downtime upgrade introduced earlier?
-* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR otherwise need attention when creating release notes? Things to consider:
+Instructions:
+1. The PR has to be tagged with at least one of the following labels:
+ 1. `feature`
+ 2. `bugfix`
+ 3. `performance`
+ 4. `ui`
+ 5. `backward-incompat`
+ 6. `release-notes` (*)
Review comment:
Add labels:
testing
dependencies
docker
kubernetes
observability
security
code-style
extension-point
refactor
cleanup
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on a change in pull request #8455: Make the PR template simpler
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #8455:
URL: https://github.com/apache/pinot/pull/8455#discussion_r840886343
##########
File path: .github/PULL_REQUEST_TEMPLATE.md
##########
@@ -1,30 +1,15 @@
-## Description
-<!-- Add a description of your PR here.
-A good description should include pointers to an issue or design document, etc.
--->
-## Upgrade Notes
-Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
-* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR fix a zero-downtime upgrade introduced earlier?
-* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR otherwise need attention when creating release notes? Things to consider:
+Instructions:
+1. The PR has to be tagged with at least one of the following labels:
+ 1. `feature`
+ 2. `bugfix`
+ 3. `performance`
+ 4. `ui`
+ 5. `backward-incompat`
+ 6. `release-notes` (*)
+2. Remove these instructions before publishing the PR.
+
+(*) Use `release-notes` label for scenarios like:
Review comment:
I am a little reluctant to add too much text to this at this time. Remember, we can always modify it going forward. Let us own the file and improve as we move forward.
That said, if the committers feel that more text is better, sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] KKcorps commented on a change in pull request #8455: Make the PR template simpler
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8455:
URL: https://github.com/apache/pinot/pull/8455#discussion_r840315294
##########
File path: .github/PULL_REQUEST_TEMPLATE.md
##########
@@ -1,30 +1,15 @@
-## Description
-<!-- Add a description of your PR here.
-A good description should include pointers to an issue or design document, etc.
--->
-## Upgrade Notes
-Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
-* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR fix a zero-downtime upgrade introduced earlier?
-* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR otherwise need attention when creating release notes? Things to consider:
+Instructions:
+1. The PR has to be tagged with at least one of the following labels:
+ 1. `feature`
+ 2. `bugfix`
+ 3. `performance`
+ 4. `ui`
+ 5. `backward-incompat`
+ 6. `release-notes` (*)
+2. Remove these instructions before publishing the PR.
+
+(*) Use `release-notes` label for scenarios like:
Review comment:
Can we but description like these for all the labels mentioned in step 1. Most of the labels are self-explanatory but we can avoid their abuse by limiting the scope for each.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on pull request #8455: Make the PR template simpler
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #8455:
URL: https://github.com/apache/pinot/pull/8455#issuecomment-1086277822
Thanks for starting this, @sajjad-moradi . The original set of instructions/checkbox/description was meant for people to apply the right label to _every_ PR, but clearly fell short of its goals. I am totally supportive of this, in any language to help achieve that goal. It is a pity if committers have to spend lots of time going through individual commits. (soon, we will not find any volunteers :)
As committers, can we agree to ensure that the PRs we merge follow these instructions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] KKcorps commented on a change in pull request #8455: Make the PR template simpler
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8455:
URL: https://github.com/apache/pinot/pull/8455#discussion_r840936768
##########
File path: .github/PULL_REQUEST_TEMPLATE.md
##########
@@ -1,30 +1,15 @@
-## Description
-<!-- Add a description of your PR here.
-A good description should include pointers to an issue or design document, etc.
--->
-## Upgrade Notes
-Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
-* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR fix a zero-downtime upgrade introduced earlier?
-* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR otherwise need attention when creating release notes? Things to consider:
+Instructions:
+1. The PR has to be tagged with at least one of the following labels:
+ 1. `feature`
+ 2. `bugfix`
+ 3. `performance`
+ 4. `ui`
+ 5. `backward-incompat`
+ 6. `release-notes` (*)
+2. Remove these instructions before publishing the PR.
+
+(*) Use `release-notes` label for scenarios like:
Review comment:
I was only talking about mentioning that unit-tests should be there. But yeah, we can decide on that and add it later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8455: Make the PR template simpler
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8455:
URL: https://github.com/apache/pinot/pull/8455#discussion_r840333733
##########
File path: .github/PULL_REQUEST_TEMPLATE.md
##########
@@ -1,30 +1,15 @@
-## Description
-<!-- Add a description of your PR here.
-A good description should include pointers to an issue or design document, etc.
--->
-## Upgrade Notes
-Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
-* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR fix a zero-downtime upgrade introduced earlier?
-* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR otherwise need attention when creating release notes? Things to consider:
+Instructions:
+1. The PR has to be tagged with at least one of the following labels:
+ 1. `feature`
+ 2. `bugfix`
+ 3. `performance`
+ 4. `ui`
+ 5. `backward-incompat`
+ 6. `release-notes` (*)
+2. Remove these instructions before publishing the PR.
+
+(*) Use `release-notes` label for scenarios like:
Review comment:
Since it’s an Apache project contributors can’t apply the labels, so it’s up to committers to do it, so “compulsory” might be off putting to first time contributors. If the author is a committer they should do it, otherwise the reviewer does it. It’s compulsory for committers, but that’s a small group of people do it’s hard not to get the memo.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #8455: Make the PR template simpler
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #8455:
URL: https://github.com/apache/pinot/pull/8455#issuecomment-1085528410
This is great, the labels can also be used to drive github actions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8455: Make the PR template simpler
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8455:
URL: https://github.com/apache/pinot/pull/8455#discussion_r841082023
##########
File path: .github/PULL_REQUEST_TEMPLATE.md
##########
@@ -1,30 +1,15 @@
-## Description
-<!-- Add a description of your PR here.
-A good description should include pointers to an issue or design document, etc.
--->
-## Upgrade Notes
-Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
-* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR fix a zero-downtime upgrade introduced earlier?
-* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
-
-Does this PR otherwise need attention when creating release notes? Things to consider:
+Instructions:
+1. The PR has to be tagged with at least one of the following labels:
+ 1. `feature`
+ 2. `bugfix`
+ 3. `performance`
+ 4. `ui`
+ 5. `backward-incompat`
+ 6. `release-notes` (*)
Review comment:
I agree, the actual problem being solved here is PRs being merged without labeling. It isn't a contributor problem. So I would argue against having a template at all (the list of labels in the template would either be incomplete or very long), and instead document how PRs should be labeled in the contribution guidelines.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org