Re: Commit Review Policy
Agree, 3 days is nothing. Am 2022-02-03 um 12:23 schrieb Xeno Amess: 3 days? according to my experience, usually you need 30 - 180 days. Tibor Digana 于2022年1月28日周五 06:40写道： It's nice to write some rules but still the developers are not machines. You can, for instance, get some vote for totally crazy things like removing public method in a library which is widely used in ASF or in the world. Yes, your right against the rules but was this according to the best practices, those best practices which must be learned by the developers for years? Was the public method @Deprecated before removal? Did you check the consumers of this artifact in the Maven repository and checked or asked the projects if it can be removed? Did you announce the community on the site? What if there are other deprecated methods and they have survived several releases but still not removed, and this method is going to be removed without deprecation? Is it consistent in project which is used by others? These are all the things which cannot be written in the rules but we have it somewhere in our mind. Do you obey your internal rules? Which have higher priority? I don't need to have an answer for my questions, since they are not questions... T On Tue, Jan 25, 2022 at 8:46 PM Slawomir Jaranowski < s.jaranow...@gmail.com> wrote: Hi, On the page "Apache Maven Project Roles" [1] we have paragraph about Committers with: The Apache Maven project uses a Commit then Review policy and has a number of conventions which should be followed. Looks like Review then Commit policy is from svn time, so should be refreshed or confirmed that is actual. When we want Review than Commit policy, we need some rules which allow us to effectively work if nobody is interested for feedback. We also need rules / examples for direct commits, when they are acceptable. Do we need different rules for Maven core, plugins, shared ... etc Example of rule: PR -> no feedback for X days -> send a mail on dev@, if still not feedback after X days after mail -> proceed alone [1] https://maven.apache.org/project-roles.html#committers -- Sławomir Jaranowski
Re: Commit Review Policy
Hi Xeno, The openjdk would close such a PR which has no attention. I am keeping it open if it is not necessary to close it, and I am doing it because I respect the time the contributor has spent on it and I want to give him a chance to continue or open a discussion. Yesterday I closed one old PR because Maven 3.0.5 is obsolete which is proven on our pages, it was necessary to close it. <- corner case!!! IMHO, the rules for people should be written very slowly, they should be verified in practice step by step. For sure, if I have to write them, I would rather start with one border/corner case and not many cases. The rule "wait 3 days" would imply that the people would absent from discussions and it would be legal, the quality would go down. So we have to decide whether we need to have features regardless of quality, or we want to have features and quality. Cheers Tibor On Fri, Feb 4, 2022 at 12:57 PM Xeno Amess wrote: > things in openjdk is if a pr cannot gain interest from any already-in > members, although it might be a great pr, it is automatically-closed. > > > There cannot be a rule "No review" -> "wait 3 days" -> "accept" because > this is the thing everybody would utilize to involve a trojan horse. > > yep, at least one apache-committer's review is needed I think. > > And apache-committer is hard to become, as I myself even not being one. > > Tibor Digana 于2022年2月4日周五 19:46写道： > > > Slawomir, we have a fundamental problem. > > You want to accept PR. But I say that the purpose of PR is not to accept > it > > because the ASFshould be always critical and therefore I think the rules > > cannot be written in terms "When to accept the PR". > > There should be rules "When not to accept the PR" because we should be > > critical to the PRs - that's the purpose of the existence of > pull-requests. > > If there is no review, no comment, most probably the ASF developers > should > > be announced, and/or it means that the business feature presented in the > PR > > is not needed. > > > > The ASF is responsible for the changes, not the contributor. > > It is no real situation to search the contributor on the plant and ask > > her/him to repair the fix which will cause a new bug in the future - we > are > > responsible, the contributor is not. > > So we should not wait for at least one approval. > > We should be critical to the PRs, and if there is one -1 it means that > the > > PR should be open as a work in progress or it should be closed. > > > > There cannot be a rule "No review" -> "wait 3 days" -> "accept" because > > this is the thing everybody would utilize to involve a trojan horse. > > > > T > > > > > > > > On Fri, Feb 4, 2022 at 12:12 PM Slawomir Jaranowski < > > s.jaranow...@gmail.com> > > wrote: > > > > > Hi, > > > Example value as 3 days or other values are to discuss and can be an > > agreed > > > value. Now such values are not important ... it will be the last item > to > > > confirm. > > > > > > I only want to show how the process can look. > > > > > > Currently we only have sentence that we use "Commit then Review" policy > > > without more details > > > > > > pt., 4 lut 2022 o 11:58 Xeno Amess napisał(a): > > > > > > > Yes, reviewing prs is time consuming, though usually worth it. > > > > 3 days does not seem enough for normal pr reviews I think. > > > > If a maintainer disagrees with this and they do think they can review > > > every > > > > prs inside the 3 days limit, then I will be glad to show him why he > > > can't, > > > > just tell me what repos he maintains. > > > > I do have the ability (and experience) in creating 100+ positively > > > > meaningful prs per day. > > > > > > > > > > > > Tibor Digana 于2022年2月4日周五 18:00写道： > > > > > > > > > IMHO the reactions against PRs should be technically critical. > > > > > IMHO the PRs from contributors should not be accepted unless there > > are > > > > > objections or pending objections. > > > > > Example, would you move your hand up and accept long commit in a PR > > > even > > > > if > > > > > you do not see the following statements? > > > > > *if ( cha[] == char[] )* > > > > > Sometimes, you need to open the PR in your IDE because GH view is > > pure > > > > for > > > > > complex changes. You should do everything in order to understand > the > > PR > > > > and > > > > > you must be convinced about the proposals almost like you were the > > > author > > > > > of the PR even if you are not the author. > > > > > > > > > > > > > > > On Thu, Feb 3, 2022 at 12:23 PM Xeno Amess > > > wrote: > > > > > > > > > > > 3 days? > > > > > > according to my experience, usually you need 30 - 180 days. > > > > > > > > > > > > Tibor Digana 于2022年1月28日周五 06:40写道： > > > > > > > > > > > > > It's nice to write some rules but still the developers are not > > > > > machines. > > > > > > > You can, for instance, get some vote for totally crazy things > > like > > > > > > removing > > > > > > > public method in a library which is widely used in ASF or in > the >
Re: Commit Review Policy
things in openjdk is if a pr cannot gain interest from any already-in members, although it might be a great pr, it is automatically-closed. > There cannot be a rule "No review" -> "wait 3 days" -> "accept" because this is the thing everybody would utilize to involve a trojan horse. yep, at least one apache-committer's review is needed I think. And apache-committer is hard to become, as I myself even not being one. Tibor Digana 于2022年2月4日周五 19:46写道： > Slawomir, we have a fundamental problem. > You want to accept PR. But I say that the purpose of PR is not to accept it > because the ASFshould be always critical and therefore I think the rules > cannot be written in terms "When to accept the PR". > There should be rules "When not to accept the PR" because we should be > critical to the PRs - that's the purpose of the existence of pull-requests. > If there is no review, no comment, most probably the ASF developers should > be announced, and/or it means that the business feature presented in the PR > is not needed. > > The ASF is responsible for the changes, not the contributor. > It is no real situation to search the contributor on the plant and ask > her/him to repair the fix which will cause a new bug in the future - we are > responsible, the contributor is not. > So we should not wait for at least one approval. > We should be critical to the PRs, and if there is one -1 it means that the > PR should be open as a work in progress or it should be closed. > > There cannot be a rule "No review" -> "wait 3 days" -> "accept" because > this is the thing everybody would utilize to involve a trojan horse. > > T > > > > On Fri, Feb 4, 2022 at 12:12 PM Slawomir Jaranowski < > s.jaranow...@gmail.com> > wrote: > > > Hi, > > Example value as 3 days or other values are to discuss and can be an > agreed > > value. Now such values are not important ... it will be the last item to > > confirm. > > > > I only want to show how the process can look. > > > > Currently we only have sentence that we use "Commit then Review" policy > > without more details > > > > pt., 4 lut 2022 o 11:58 Xeno Amess napisał(a): > > > > > Yes, reviewing prs is time consuming, though usually worth it. > > > 3 days does not seem enough for normal pr reviews I think. > > > If a maintainer disagrees with this and they do think they can review > > every > > > prs inside the 3 days limit, then I will be glad to show him why he > > can't, > > > just tell me what repos he maintains. > > > I do have the ability (and experience) in creating 100+ positively > > > meaningful prs per day. > > > > > > > > > Tibor Digana 于2022年2月4日周五 18:00写道： > > > > > > > IMHO the reactions against PRs should be technically critical. > > > > IMHO the PRs from contributors should not be accepted unless there > are > > > > objections or pending objections. > > > > Example, would you move your hand up and accept long commit in a PR > > even > > > if > > > > you do not see the following statements? > > > > *if ( cha[] == char[] )* > > > > Sometimes, you need to open the PR in your IDE because GH view is > pure > > > for > > > > complex changes. You should do everything in order to understand the > PR > > > and > > > > you must be convinced about the proposals almost like you were the > > author > > > > of the PR even if you are not the author. > > > > > > > > > > > > On Thu, Feb 3, 2022 at 12:23 PM Xeno Amess > > wrote: > > > > > > > > > 3 days? > > > > > according to my experience, usually you need 30 - 180 days. > > > > > > > > > > Tibor Digana 于2022年1月28日周五 06:40写道： > > > > > > > > > > > It's nice to write some rules but still the developers are not > > > > machines. > > > > > > You can, for instance, get some vote for totally crazy things > like > > > > > removing > > > > > > public method in a library which is widely used in ASF or in the > > > world. > > > > > > Yes, your right against the rules but was this according to the > > best > > > > > > practices, those best practices which must be learned by the > > > developers > > > > > for > > > > > > years? > > > > > > Was the public method @Deprecated before removal? > > > > > > Did you check the consumers of this artifact in the Maven > > repository > > > > and > > > > > > checked or asked the projects if it can be removed? > > > > > > Did you announce the community on the site? > > > > > > What if there are other deprecated methods and they have survived > > > > several > > > > > > releases but still not removed, and this method is going to be > > > removed > > > > > > without deprecation? Is it consistent in project which is used by > > > > others? > > > > > > These are all the things which cannot be written in the rules but > > we > > > > have > > > > > > it somewhere in our mind. > > > > > > Do you obey your internal rules? > > > > > > Which have higher priority? > > > > > > > > > > > > I don't need to have an answer for my questions, since they are > not > > > > > > questions... > > > > > > > > > > > > T > > > > >
Re: Commit Review Policy
Slawomir, we have a fundamental problem. You want to accept PR. But I say that the purpose of PR is not to accept it because the ASFshould be always critical and therefore I think the rules cannot be written in terms "When to accept the PR". There should be rules "When not to accept the PR" because we should be critical to the PRs - that's the purpose of the existence of pull-requests. If there is no review, no comment, most probably the ASF developers should be announced, and/or it means that the business feature presented in the PR is not needed. The ASF is responsible for the changes, not the contributor. It is no real situation to search the contributor on the plant and ask her/him to repair the fix which will cause a new bug in the future - we are responsible, the contributor is not. So we should not wait for at least one approval. We should be critical to the PRs, and if there is one -1 it means that the PR should be open as a work in progress or it should be closed. There cannot be a rule "No review" -> "wait 3 days" -> "accept" because this is the thing everybody would utilize to involve a trojan horse. T On Fri, Feb 4, 2022 at 12:12 PM Slawomir Jaranowski wrote: > Hi, > Example value as 3 days or other values are to discuss and can be an agreed > value. Now such values are not important ... it will be the last item to > confirm. > > I only want to show how the process can look. > > Currently we only have sentence that we use "Commit then Review" policy > without more details > > pt., 4 lut 2022 o 11:58 Xeno Amess napisał(a): > > > Yes, reviewing prs is time consuming, though usually worth it. > > 3 days does not seem enough for normal pr reviews I think. > > If a maintainer disagrees with this and they do think they can review > every > > prs inside the 3 days limit, then I will be glad to show him why he > can't, > > just tell me what repos he maintains. > > I do have the ability (and experience) in creating 100+ positively > > meaningful prs per day. > > > > > > Tibor Digana 于2022年2月4日周五 18:00写道： > > > > > IMHO the reactions against PRs should be technically critical. > > > IMHO the PRs from contributors should not be accepted unless there are > > > objections or pending objections. > > > Example, would you move your hand up and accept long commit in a PR > even > > if > > > you do not see the following statements? > > > *if ( cha[] == char[] )* > > > Sometimes, you need to open the PR in your IDE because GH view is pure > > for > > > complex changes. You should do everything in order to understand the PR > > and > > > you must be convinced about the proposals almost like you were the > author > > > of the PR even if you are not the author. > > > > > > > > > On Thu, Feb 3, 2022 at 12:23 PM Xeno Amess > wrote: > > > > > > > 3 days? > > > > according to my experience, usually you need 30 - 180 days. > > > > > > > > Tibor Digana 于2022年1月28日周五 06:40写道： > > > > > > > > > It's nice to write some rules but still the developers are not > > > machines. > > > > > You can, for instance, get some vote for totally crazy things like > > > > removing > > > > > public method in a library which is widely used in ASF or in the > > world. > > > > > Yes, your right against the rules but was this according to the > best > > > > > practices, those best practices which must be learned by the > > developers > > > > for > > > > > years? > > > > > Was the public method @Deprecated before removal? > > > > > Did you check the consumers of this artifact in the Maven > repository > > > and > > > > > checked or asked the projects if it can be removed? > > > > > Did you announce the community on the site? > > > > > What if there are other deprecated methods and they have survived > > > several > > > > > releases but still not removed, and this method is going to be > > removed > > > > > without deprecation? Is it consistent in project which is used by > > > others? > > > > > These are all the things which cannot be written in the rules but > we > > > have > > > > > it somewhere in our mind. > > > > > Do you obey your internal rules? > > > > > Which have higher priority? > > > > > > > > > > I don't need to have an answer for my questions, since they are not > > > > > questions... > > > > > > > > > > T > > > > > > > > > > On Tue, Jan 25, 2022 at 8:46 PM Slawomir Jaranowski < > > > > > s.jaranow...@gmail.com> > > > > > wrote: > > > > > > > > > > > Hi, > > > > > > > > > > > > On the page "Apache Maven Project Roles" [1] we have paragraph > > > > > > about Committers with: > > > > > > > > > > > > The Apache Maven project uses a Commit then Review policy and > has a > > > > > number > > > > > > of conventions which should be followed. > > > > > > > > > > > > > > > > > > Looks like Review then Commit policy is from svn time, so should > be > > > > > > refreshed or confirmed that is actual. > > > > > > > > > > > > When we want Review than Commit policy, we need some rules which > > > allow > > > > us > > > > > > to
Re: Commit Review Policy
Hi, Example value as 3 days or other values are to discuss and can be an agreed value. Now such values are not important ... it will be the last item to confirm. I only want to show how the process can look. Currently we only have sentence that we use "Commit then Review" policy without more details pt., 4 lut 2022 o 11:58 Xeno Amess napisał(a): > Yes, reviewing prs is time consuming, though usually worth it. > 3 days does not seem enough for normal pr reviews I think. > If a maintainer disagrees with this and they do think they can review every > prs inside the 3 days limit, then I will be glad to show him why he can't, > just tell me what repos he maintains. > I do have the ability (and experience) in creating 100+ positively > meaningful prs per day. > > > Tibor Digana 于2022年2月4日周五 18:00写道： > > > IMHO the reactions against PRs should be technically critical. > > IMHO the PRs from contributors should not be accepted unless there are > > objections or pending objections. > > Example, would you move your hand up and accept long commit in a PR even > if > > you do not see the following statements? > > *if ( cha[] == char[] )* > > Sometimes, you need to open the PR in your IDE because GH view is pure > for > > complex changes. You should do everything in order to understand the PR > and > > you must be convinced about the proposals almost like you were the author > > of the PR even if you are not the author. > > > > > > On Thu, Feb 3, 2022 at 12:23 PM Xeno Amess wrote: > > > > > 3 days? > > > according to my experience, usually you need 30 - 180 days. > > > > > > Tibor Digana 于2022年1月28日周五 06:40写道： > > > > > > > It's nice to write some rules but still the developers are not > > machines. > > > > You can, for instance, get some vote for totally crazy things like > > > removing > > > > public method in a library which is widely used in ASF or in the > world. > > > > Yes, your right against the rules but was this according to the best > > > > practices, those best practices which must be learned by the > developers > > > for > > > > years? > > > > Was the public method @Deprecated before removal? > > > > Did you check the consumers of this artifact in the Maven repository > > and > > > > checked or asked the projects if it can be removed? > > > > Did you announce the community on the site? > > > > What if there are other deprecated methods and they have survived > > several > > > > releases but still not removed, and this method is going to be > removed > > > > without deprecation? Is it consistent in project which is used by > > others? > > > > These are all the things which cannot be written in the rules but we > > have > > > > it somewhere in our mind. > > > > Do you obey your internal rules? > > > > Which have higher priority? > > > > > > > > I don't need to have an answer for my questions, since they are not > > > > questions... > > > > > > > > T > > > > > > > > On Tue, Jan 25, 2022 at 8:46 PM Slawomir Jaranowski < > > > > s.jaranow...@gmail.com> > > > > wrote: > > > > > > > > > Hi, > > > > > > > > > > On the page "Apache Maven Project Roles" [1] we have paragraph > > > > > about Committers with: > > > > > > > > > > The Apache Maven project uses a Commit then Review policy and has a > > > > number > > > > > of conventions which should be followed. > > > > > > > > > > > > > > > Looks like Review then Commit policy is from svn time, so should be > > > > > refreshed or confirmed that is actual. > > > > > > > > > > When we want Review than Commit policy, we need some rules which > > allow > > > us > > > > > to effectively work if nobody is interested for feedback. > > > > > We also need rules / examples for direct commits, when they are > > > > acceptable. > > > > > > > > > > Do we need different rules for Maven core, plugins, shared ... etc > > > > > > > > > > Example of rule: > > > > > > > > > > PR -> no feedback for X days -> send a mail on dev@, if still not > > > > feedback > > > > > after X days after mail -> proceed alone > > > > > > > > > > [1] https://maven.apache.org/project-roles.html#committers > > > > > > > > > > -- > > > > > Sławomir Jaranowski > > > > > > > > > > > > > > > -- Sławomir Jaranowski
Re: Commit Review Policy
Yes, reviewing prs is time consuming, though usually worth it. 3 days does not seem enough for normal pr reviews I think. If a maintainer disagrees with this and they do think they can review every prs inside the 3 days limit, then I will be glad to show him why he can't, just tell me what repos he maintains. I do have the ability (and experience) in creating 100+ positively meaningful prs per day. Tibor Digana 于2022年2月4日周五 18:00写道： > IMHO the reactions against PRs should be technically critical. > IMHO the PRs from contributors should not be accepted unless there are > objections or pending objections. > Example, would you move your hand up and accept long commit in a PR even if > you do not see the following statements? > *if ( cha[] == char[] )* > Sometimes, you need to open the PR in your IDE because GH view is pure for > complex changes. You should do everything in order to understand the PR and > you must be convinced about the proposals almost like you were the author > of the PR even if you are not the author. > > > On Thu, Feb 3, 2022 at 12:23 PM Xeno Amess wrote: > > > 3 days? > > according to my experience, usually you need 30 - 180 days. > > > > Tibor Digana 于2022年1月28日周五 06:40写道： > > > > > It's nice to write some rules but still the developers are not > machines. > > > You can, for instance, get some vote for totally crazy things like > > removing > > > public method in a library which is widely used in ASF or in the world. > > > Yes, your right against the rules but was this according to the best > > > practices, those best practices which must be learned by the developers > > for > > > years? > > > Was the public method @Deprecated before removal? > > > Did you check the consumers of this artifact in the Maven repository > and > > > checked or asked the projects if it can be removed? > > > Did you announce the community on the site? > > > What if there are other deprecated methods and they have survived > several > > > releases but still not removed, and this method is going to be removed > > > without deprecation? Is it consistent in project which is used by > others? > > > These are all the things which cannot be written in the rules but we > have > > > it somewhere in our mind. > > > Do you obey your internal rules? > > > Which have higher priority? > > > > > > I don't need to have an answer for my questions, since they are not > > > questions... > > > > > > T > > > > > > On Tue, Jan 25, 2022 at 8:46 PM Slawomir Jaranowski < > > > s.jaranow...@gmail.com> > > > wrote: > > > > > > > Hi, > > > > > > > > On the page "Apache Maven Project Roles" [1] we have paragraph > > > > about Committers with: > > > > > > > > The Apache Maven project uses a Commit then Review policy and has a > > > number > > > > of conventions which should be followed. > > > > > > > > > > > > Looks like Review then Commit policy is from svn time, so should be > > > > refreshed or confirmed that is actual. > > > > > > > > When we want Review than Commit policy, we need some rules which > allow > > us > > > > to effectively work if nobody is interested for feedback. > > > > We also need rules / examples for direct commits, when they are > > > acceptable. > > > > > > > > Do we need different rules for Maven core, plugins, shared ... etc > > > > > > > > Example of rule: > > > > > > > > PR -> no feedback for X days -> send a mail on dev@, if still not > > > feedback > > > > after X days after mail -> proceed alone > > > > > > > > [1] https://maven.apache.org/project-roles.html#committers > > > > > > > > -- > > > > Sławomir Jaranowski > > > > > > > > > >
Re: Commit Review Policy
IMHO the reactions against PRs should be technically critical. IMHO the PRs from contributors should not be accepted unless there are objections or pending objections. Example, would you move your hand up and accept long commit in a PR even if you do not see the following statements? *if ( cha[] == char[] )* Sometimes, you need to open the PR in your IDE because GH view is pure for complex changes. You should do everything in order to understand the PR and you must be convinced about the proposals almost like you were the author of the PR even if you are not the author. On Thu, Feb 3, 2022 at 12:23 PM Xeno Amess wrote: > 3 days? > according to my experience, usually you need 30 - 180 days. > > Tibor Digana 于2022年1月28日周五 06:40写道： > > > It's nice to write some rules but still the developers are not machines. > > You can, for instance, get some vote for totally crazy things like > removing > > public method in a library which is widely used in ASF or in the world. > > Yes, your right against the rules but was this according to the best > > practices, those best practices which must be learned by the developers > for > > years? > > Was the public method @Deprecated before removal? > > Did you check the consumers of this artifact in the Maven repository and > > checked or asked the projects if it can be removed? > > Did you announce the community on the site? > > What if there are other deprecated methods and they have survived several > > releases but still not removed, and this method is going to be removed > > without deprecation? Is it consistent in project which is used by others? > > These are all the things which cannot be written in the rules but we have > > it somewhere in our mind. > > Do you obey your internal rules? > > Which have higher priority? > > > > I don't need to have an answer for my questions, since they are not > > questions... > > > > T > > > > On Tue, Jan 25, 2022 at 8:46 PM Slawomir Jaranowski < > > s.jaranow...@gmail.com> > > wrote: > > > > > Hi, > > > > > > On the page "Apache Maven Project Roles" [1] we have paragraph > > > about Committers with: > > > > > > The Apache Maven project uses a Commit then Review policy and has a > > number > > > of conventions which should be followed. > > > > > > > > > Looks like Review then Commit policy is from svn time, so should be > > > refreshed or confirmed that is actual. > > > > > > When we want Review than Commit policy, we need some rules which allow > us > > > to effectively work if nobody is interested for feedback. > > > We also need rules / examples for direct commits, when they are > > acceptable. > > > > > > Do we need different rules for Maven core, plugins, shared ... etc > > > > > > Example of rule: > > > > > > PR -> no feedback for X days -> send a mail on dev@, if still not > > feedback > > > after X days after mail -> proceed alone > > > > > > [1] https://maven.apache.org/project-roles.html#committers > > > > > > -- > > > Sławomir Jaranowski > > > > > >
Re: Commit Review Policy
3 days? according to my experience, usually you need 30 - 180 days. Tibor Digana 于2022年1月28日周五 06:40写道： > It's nice to write some rules but still the developers are not machines. > You can, for instance, get some vote for totally crazy things like removing > public method in a library which is widely used in ASF or in the world. > Yes, your right against the rules but was this according to the best > practices, those best practices which must be learned by the developers for > years? > Was the public method @Deprecated before removal? > Did you check the consumers of this artifact in the Maven repository and > checked or asked the projects if it can be removed? > Did you announce the community on the site? > What if there are other deprecated methods and they have survived several > releases but still not removed, and this method is going to be removed > without deprecation? Is it consistent in project which is used by others? > These are all the things which cannot be written in the rules but we have > it somewhere in our mind. > Do you obey your internal rules? > Which have higher priority? > > I don't need to have an answer for my questions, since they are not > questions... > > T > > On Tue, Jan 25, 2022 at 8:46 PM Slawomir Jaranowski < > s.jaranow...@gmail.com> > wrote: > > > Hi, > > > > On the page "Apache Maven Project Roles" [1] we have paragraph > > about Committers with: > > > > The Apache Maven project uses a Commit then Review policy and has a > number > > of conventions which should be followed. > > > > > > Looks like Review then Commit policy is from svn time, so should be > > refreshed or confirmed that is actual. > > > > When we want Review than Commit policy, we need some rules which allow us > > to effectively work if nobody is interested for feedback. > > We also need rules / examples for direct commits, when they are > acceptable. > > > > Do we need different rules for Maven core, plugins, shared ... etc > > > > Example of rule: > > > > PR -> no feedback for X days -> send a mail on dev@, if still not > feedback > > after X days after mail -> proceed alone > > > > [1] https://maven.apache.org/project-roles.html#committers > > > > -- > > Sławomir Jaranowski > > >
Re: Commit Review Policy
It's nice to write some rules but still the developers are not machines. You can, for instance, get some vote for totally crazy things like removing public method in a library which is widely used in ASF or in the world. Yes, your right against the rules but was this according to the best practices, those best practices which must be learned by the developers for years? Was the public method @Deprecated before removal? Did you check the consumers of this artifact in the Maven repository and checked or asked the projects if it can be removed? Did you announce the community on the site? What if there are other deprecated methods and they have survived several releases but still not removed, and this method is going to be removed without deprecation? Is it consistent in project which is used by others? These are all the things which cannot be written in the rules but we have it somewhere in our mind. Do you obey your internal rules? Which have higher priority? I don't need to have an answer for my questions, since they are not questions... T On Tue, Jan 25, 2022 at 8:46 PM Slawomir Jaranowski wrote: > Hi, > > On the page "Apache Maven Project Roles" [1] we have paragraph > about Committers with: > > The Apache Maven project uses a Commit then Review policy and has a number > of conventions which should be followed. > > > Looks like Review then Commit policy is from svn time, so should be > refreshed or confirmed that is actual. > > When we want Review than Commit policy, we need some rules which allow us > to effectively work if nobody is interested for feedback. > We also need rules / examples for direct commits, when they are acceptable. > > Do we need different rules for Maven core, plugins, shared ... etc > > Example of rule: > > PR -> no feedback for X days -> send a mail on dev@, if still not feedback > after X days after mail -> proceed alone > > [1] https://maven.apache.org/project-roles.html#committers > > -- > Sławomir Jaranowski >
Re: Commit Review Policy
Am 2022-01-26 um 22:00 schrieb Slawomir Jaranowski: Hi, What do you think ... Maven Core and important components (list needed) - RTC obligatory, approve from X committers, no request changes Good. Common rules: - build and tests always pass or are under control - after each commit project is ready to next release - as maintainer add page https://github.com/pulls/review-requested to your favorite Insane, I did not even know about this overview. Recommended changes without PR: - fix links in docs - fix misspells - code comments - improve for configuration, like .asf.yaml, .gitignore, GitHub workflows, etc - another Agreed. Rest - CTR with recommended PR with rules 1. No review - PR - create - request review by GitHub from at least 2 commiers - wait 3 days (the same as for votes or another value) - at least one approve or no response - process At least three business days, not calendar days. But also consider that even non-core can have a lot implication if it is a important component. I don't know whether 3 days is enough. I need often much more than three days. Consider that almost everyone here is a volunteer. 2. Review with comments - PR - commented without request changes - consider and make suggested fix, response for question - rerequest review on GitHub - wait 3 days - process As above. 3. Review with request changes - PR commented with request changes - make suggested fix or explanation, response for question - re request review on GitHub - at least one approve from someone else, or remove request changes by author - process As above. The expected behavior at the invitation for review - do review in 3 days - leave comments that you want to review later, point when - drop yourself from review request if don't want to do review - add someone else for review request Makes sense. Of course each change is unique and the author knows the best if should wait for review and the author is aware and responsible for self change. +2 no this one. Think twice whether you are 100% certain that your change pushed unreviewed. śr., 26 sty 2022 o 10:34 Olivier Lamy napisał(a): makes sense for core and it looks to be a de facto standard. but plugins and components maintained by only 1 (maybe 2) persons. we already have plenty of PRs from external contributors never reviewed... And I do not mention some PRs which take weeks to review.. I feel like adding this will: - ADDING EVEN MORE EMAILS TO THE ALREADY HUGE AMOUNT WE RECEIVE!!! (frankly it's already hard to follow anything and I already miss some ping) - delay even more moving forward (such process works for companies with people dedicated to it but we are volunteers) core or important components that's fine but I definitely vote against moving plugins/components to RTC . On Wed, 26 Jan 2022 at 19:19, Enrico Olivelli wrote: Il giorno mer 26 gen 2022 alle ore 07:44 Benjamin Marwell ha scritto: Hi! Aside from typo fixes, PRs might slow down the dev speed, but greatly improve code quality. Even small PRs might have side effects you might not notice at first glance. But then, even with typo fixes I have seen people introducing more fixes or unintentionally thought they would fix something when in fact they introduced a type. In my humble opinion, reviews BEFORE commit should be the default case. - review from at least 2 committers (so only one if the patch comes from a committer) - CI passing +1 Enrico Am Di., 25. Jan. 2022 um 20:47 Uhr schrieb Slawomir Jaranowski : Hi, On the page "Apache Maven Project Roles" [1] we have paragraph about Committers with: The Apache Maven project uses a Commit then Review policy and has a number of conventions which should be followed. Looks like Review then Commit policy is from svn time, so should be refreshed or confirmed that is actual. When we want Review than Commit policy, we need some rules which allow us to effectively work if nobody is interested for feedback. We also need rules / examples for direct commits, when they are acceptable. Do we need different rules for Maven core, plugins, shared ... etc Example of rule: PR -> no feedback for X days -> send a mail on dev@, if still not feedback after X days after mail -> proceed alone [1] https://maven.apache.org/project-roles.html#committers -- Sławomir Jaranowski - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
Re: Commit Review Policy
Hi, What do you think ... Maven Core and important components (list needed) - RTC obligatory, approve from X committers, no request changes Common rules: - build and tests always pass or are under control - after each commit project is ready to next release - as maintainer add page https://github.com/pulls/review-requested to your favorite Recommended changes without PR: - fix links in docs - fix misspells - code comments - improve for configuration, like .asf.yaml, .gitignore, GitHub workflows, etc - another Rest - CTR with recommended PR with rules 1. No review - PR - create - request review by GitHub from at least 2 commiers - wait 3 days (the same as for votes or another value) - at least one approve or no response - process 2. Review with comments - PR - commented without request changes - consider and make suggested fix, response for question - rerequest review on GitHub - wait 3 days - process 3. Review with request changes - PR commented with request changes - make suggested fix or explanation, response for question - re request review on GitHub - at least one approve from someone else, or remove request changes by author - process The expected behavior at the invitation for review - do review in 3 days - leave comments that you want to review later, point when - drop yourself from review request if don't want to do review - add someone else for review request Of course each change is unique and the author knows the best if should wait for review and the author is aware and responsible for self change. śr., 26 sty 2022 o 10:34 Olivier Lamy napisał(a): > makes sense for core and it looks to be a de facto standard. > but plugins and components maintained by only 1 (maybe 2) persons. > we already have plenty of PRs from external contributors never reviewed... > And I do not mention some PRs which take weeks to review.. > I feel like adding this will: > - ADDING EVEN MORE EMAILS TO THE ALREADY HUGE AMOUNT WE RECEIVE!!! (frankly > it's already hard to follow anything and I already miss some ping) > - delay even more moving forward (such process works for companies with > people dedicated to it but we are volunteers) > > core or important components that's fine but I definitely vote against > moving plugins/components to RTC > > . > > On Wed, 26 Jan 2022 at 19:19, Enrico Olivelli wrote: > > > Il giorno mer 26 gen 2022 alle ore 07:44 Benjamin Marwell > > ha scritto: > > > > > > Hi! > > > > > > Aside from typo fixes, PRs might slow down the dev speed, but greatly > > > improve code quality. > > > Even small PRs might have side effects you might not notice at first > > glance. > > > > > > But then, even with typo fixes I have seen people introducing more > > > fixes or unintentionally > > > thought they would fix something when in fact they introduced a type. > > > > > > In my humble opinion, reviews BEFORE commit should be the default case. > > > > - review from at least 2 committers (so only one if the patch comes > > from a committer) > > - CI passing > > > > > > > > +1 > > > > Enrico > > > > > > > > Am Di., 25. Jan. 2022 um 20:47 Uhr schrieb Slawomir Jaranowski > > > : > > > > > > > > Hi, > > > > > > > > On the page "Apache Maven Project Roles" [1] we have paragraph > > > > about Committers with: > > > > > > > > The Apache Maven project uses a Commit then Review policy and has a > > number > > > > of conventions which should be followed. > > > > > > > > > > > > Looks like Review then Commit policy is from svn time, so should be > > > > refreshed or confirmed that is actual. > > > > > > > > When we want Review than Commit policy, we need some rules which > allow > > us > > > > to effectively work if nobody is interested for feedback. > > > > We also need rules / examples for direct commits, when they are > > acceptable. > > > > > > > > Do we need different rules for Maven core, plugins, shared ... etc > > > > > > > > Example of rule: > > > > > > > > PR -> no feedback for X days -> send a mail on dev@, if still not > > feedback > > > > after X days after mail -> proceed alone > > > > > > > > [1] https://maven.apache.org/project-roles.html#committers > > > > > > > > -- > > > > Sławomir Jaranowski > > > > > > - > > > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > > > For additional commands, e-mail: dev-h...@maven.apache.org > > > > > > > - > > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > > For additional commands, e-mail: dev-h...@maven.apache.org > > > > > -- Sławomir Jaranowski
Re: Commit Review Policy
makes sense for core and it looks to be a de facto standard. but plugins and components maintained by only 1 (maybe 2) persons. we already have plenty of PRs from external contributors never reviewed... And I do not mention some PRs which take weeks to review.. I feel like adding this will: - ADDING EVEN MORE EMAILS TO THE ALREADY HUGE AMOUNT WE RECEIVE!!! (frankly it's already hard to follow anything and I already miss some ping) - delay even more moving forward (such process works for companies with people dedicated to it but we are volunteers) core or important components that's fine but I definitely vote against moving plugins/components to RTC . On Wed, 26 Jan 2022 at 19:19, Enrico Olivelli wrote: > Il giorno mer 26 gen 2022 alle ore 07:44 Benjamin Marwell > ha scritto: > > > > Hi! > > > > Aside from typo fixes, PRs might slow down the dev speed, but greatly > > improve code quality. > > Even small PRs might have side effects you might not notice at first > glance. > > > > But then, even with typo fixes I have seen people introducing more > > fixes or unintentionally > > thought they would fix something when in fact they introduced a type. > > > > In my humble opinion, reviews BEFORE commit should be the default case. > > - review from at least 2 committers (so only one if the patch comes > from a committer) > - CI passing > > > > +1 > > Enrico > > > > > Am Di., 25. Jan. 2022 um 20:47 Uhr schrieb Slawomir Jaranowski > > : > > > > > > Hi, > > > > > > On the page "Apache Maven Project Roles" [1] we have paragraph > > > about Committers with: > > > > > > The Apache Maven project uses a Commit then Review policy and has a > number > > > of conventions which should be followed. > > > > > > > > > Looks like Review then Commit policy is from svn time, so should be > > > refreshed or confirmed that is actual. > > > > > > When we want Review than Commit policy, we need some rules which allow > us > > > to effectively work if nobody is interested for feedback. > > > We also need rules / examples for direct commits, when they are > acceptable. > > > > > > Do we need different rules for Maven core, plugins, shared ... etc > > > > > > Example of rule: > > > > > > PR -> no feedback for X days -> send a mail on dev@, if still not > feedback > > > after X days after mail -> proceed alone > > > > > > [1] https://maven.apache.org/project-roles.html#committers > > > > > > -- > > > Sławomir Jaranowski > > > > - > > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > > For additional commands, e-mail: dev-h...@maven.apache.org > > > > - > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > For additional commands, e-mail: dev-h...@maven.apache.org > >
Re: Commit Review Policy
Howdy, In your example rule, between 1st and 2nd step I'd insert some "invite interested parties" step, that is either request review using GH UI, or ping party via mail... As currently our incoming email amount is huge, it is way too easy to miss some. Also, when asked for review, the one asked by going to https://github.com/pulls can easily glean over where the review is needed. T On Tue, Jan 25, 2022 at 8:47 PM Slawomir Jaranowski wrote: > Hi, > > On the page "Apache Maven Project Roles" [1] we have paragraph > about Committers with: > > The Apache Maven project uses a Commit then Review policy and has a number > of conventions which should be followed. > > > Looks like Review then Commit policy is from svn time, so should be > refreshed or confirmed that is actual. > > When we want Review than Commit policy, we need some rules which allow us > to effectively work if nobody is interested for feedback. > We also need rules / examples for direct commits, when they are acceptable. > > Do we need different rules for Maven core, plugins, shared ... etc > > Example of rule: > > PR -> no feedback for X days -> send a mail on dev@, if still not feedback > after X days after mail -> proceed alone > > [1] https://maven.apache.org/project-roles.html#committers > > -- > Sławomir Jaranowski >
Re: Commit Review Policy
Il giorno mer 26 gen 2022 alle ore 07:44 Benjamin Marwell ha scritto: > > Hi! > > Aside from typo fixes, PRs might slow down the dev speed, but greatly > improve code quality. > Even small PRs might have side effects you might not notice at first glance. > > But then, even with typo fixes I have seen people introducing more > fixes or unintentionally > thought they would fix something when in fact they introduced a type. > > In my humble opinion, reviews BEFORE commit should be the default case. - review from at least 2 committers (so only one if the patch comes from a committer) - CI passing +1 Enrico > > Am Di., 25. Jan. 2022 um 20:47 Uhr schrieb Slawomir Jaranowski > : > > > > Hi, > > > > On the page "Apache Maven Project Roles" [1] we have paragraph > > about Committers with: > > > > The Apache Maven project uses a Commit then Review policy and has a number > > of conventions which should be followed. > > > > > > Looks like Review then Commit policy is from svn time, so should be > > refreshed or confirmed that is actual. > > > > When we want Review than Commit policy, we need some rules which allow us > > to effectively work if nobody is interested for feedback. > > We also need rules / examples for direct commits, when they are acceptable. > > > > Do we need different rules for Maven core, plugins, shared ... etc > > > > Example of rule: > > > > PR -> no feedback for X days -> send a mail on dev@, if still not feedback > > after X days after mail -> proceed alone > > > > [1] https://maven.apache.org/project-roles.html#committers > > > > -- > > Sławomir Jaranowski > > - > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > For additional commands, e-mail: dev-h...@maven.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
Re: Commit Review Policy
Hi! Aside from typo fixes, PRs might slow down the dev speed, but greatly improve code quality. Even small PRs might have side effects you might not notice at first glance. But then, even with typo fixes I have seen people introducing more fixes or unintentionally thought they would fix something when in fact they introduced a type. In my humble opinion, reviews BEFORE commit should be the default case. Am Di., 25. Jan. 2022 um 20:47 Uhr schrieb Slawomir Jaranowski : > > Hi, > > On the page "Apache Maven Project Roles" [1] we have paragraph > about Committers with: > > The Apache Maven project uses a Commit then Review policy and has a number > of conventions which should be followed. > > > Looks like Review then Commit policy is from svn time, so should be > refreshed or confirmed that is actual. > > When we want Review than Commit policy, we need some rules which allow us > to effectively work if nobody is interested for feedback. > We also need rules / examples for direct commits, when they are acceptable. > > Do we need different rules for Maven core, plugins, shared ... etc > > Example of rule: > > PR -> no feedback for X days -> send a mail on dev@, if still not feedback > after X days after mail -> proceed alone > > [1] https://maven.apache.org/project-roles.html#committers > > -- > Sławomir Jaranowski - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
Re: Commit Review Policy
Hi, I'm happy with the current rule/process CTR and not sure we want to receive more notification emails... when someone creates a PR to fix a typo in some documentation/javadoc or some very easy changes. We have enough experience to create a PR when really necessary. On Wed, 26 Jan 2022 at 05:46, Slawomir Jaranowski wrote: > Hi, > > On the page "Apache Maven Project Roles" [1] we have paragraph > about Committers with: > > The Apache Maven project uses a Commit then Review policy and has a number > of conventions which should be followed. > > > Looks like Review then Commit policy is from svn time, so should be > refreshed or confirmed that is actual. > > When we want Review than Commit policy, we need some rules which allow us > to effectively work if nobody is interested for feedback. > We also need rules / examples for direct commits, when they are acceptable. > > Do we need different rules for Maven core, plugins, shared ... etc > > Example of rule: > > PR -> no feedback for X days -> send a mail on dev@, if still not feedback > after X days after mail -> proceed alone > > [1] https://maven.apache.org/project-roles.html#committers > > -- > Sławomir Jaranowski >
