You are viewing a plain text version of this content. The canonical link for it is here.
Posted to hdfs-dev@hadoop.apache.org by Wei-Chiu Chuang <we...@apache.org> on 2021/06/02 04:43:11 UTC

Do I need a +1 to merge a backport PR?

I'm curious about the GitHub PR conventions we use today... say I want to
backport a commit from trunk to branch-3.3, and there's a small code
conflict so I push a PR against branch-3.3 using GitHub to go through the
precommit check.

Do I need explicit approval from another committer to merge the backport
PR? (As.a committer, I know I can merge at any time) or can I merge when
the precommit comes back okay?

Re: [E] Re: Do I need a +1 to merge a backport PR?

Posted by Brahma Reddy Battula <br...@apache.org>.
Currently how the addendum will be added to PR??
 Won’t  be another PR?? Where build run again ??


On Thu, 3 Jun 2021 at 1:33 AM, Eric Badger <eb...@verizonmedia.com.invalid>
wrote:

> I'm of a similar opinion to most here. If the backport is clean, I think
> it's ok to do it with just the +1 on the original patch. However, please
> please please build the code on the target branch before backporting
>
> Eric
>
> On Wed, Jun 2, 2021 at 2:46 PM Ayush Saxena <ay...@gmail.com> wrote:
>
> > For trivial changes, like changes in import or conflicts due to line
> > number or other trivial stuff, I don’t think that is required. Unless the
> > general  logic isn’t changing, we can go ahead, may be we can do a test
> run
> > before merging, to be on the safer side as and when required. :-)
> >
> > -Ayush
> >
> > > On 02-Jun-2021, at 10:13 AM, Wei-Chiu Chuang <we...@apache.org>
> wrote:
> > >
> > > I'm curious about the GitHub PR conventions we use today... say I want
> > to
> > > backport a commit from trunk to branch-3.3, and there's a small code
> > > conflict so I push a PR against branch-3.3 using GitHub to go through
> the
> > > precommit check.
> > >
> > > Do I need explicit approval from another committer to merge the
> backport
> > > PR? (As.a committer, I know I can merge at any time) or can I merge
> when
> > > the precommit comes back okay?
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: hdfs-dev-unsubscribe@hadoop.apache.org
> > For additional commands, e-mail: hdfs-dev-help@hadoop.apache.org
> >
> >
>
-- 



--Brahma Reddy Battula

Re: [E] Re: Do I need a +1 to merge a backport PR?

Posted by Brahma Reddy Battula <br...@apache.org>.
Currently how the addendum will be added to PR??
 Won’t  be another PR?? Where build run again ??


On Thu, 3 Jun 2021 at 1:33 AM, Eric Badger <eb...@verizonmedia.com.invalid>
wrote:

> I'm of a similar opinion to most here. If the backport is clean, I think
> it's ok to do it with just the +1 on the original patch. However, please
> please please build the code on the target branch before backporting
>
> Eric
>
> On Wed, Jun 2, 2021 at 2:46 PM Ayush Saxena <ay...@gmail.com> wrote:
>
> > For trivial changes, like changes in import or conflicts due to line
> > number or other trivial stuff, I don’t think that is required. Unless the
> > general  logic isn’t changing, we can go ahead, may be we can do a test
> run
> > before merging, to be on the safer side as and when required. :-)
> >
> > -Ayush
> >
> > > On 02-Jun-2021, at 10:13 AM, Wei-Chiu Chuang <we...@apache.org>
> wrote:
> > >
> > > I'm curious about the GitHub PR conventions we use today... say I want
> > to
> > > backport a commit from trunk to branch-3.3, and there's a small code
> > > conflict so I push a PR against branch-3.3 using GitHub to go through
> the
> > > precommit check.
> > >
> > > Do I need explicit approval from another committer to merge the
> backport
> > > PR? (As.a committer, I know I can merge at any time) or can I merge
> when
> > > the precommit comes back okay?
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: hdfs-dev-unsubscribe@hadoop.apache.org
> > For additional commands, e-mail: hdfs-dev-help@hadoop.apache.org
> >
> >
>
-- 



--Brahma Reddy Battula

Re: [E] Re: Do I need a +1 to merge a backport PR?

Posted by Eric Badger <eb...@verizonmedia.com.INVALID>.
I'm of a similar opinion to most here. If the backport is clean, I think
it's ok to do it with just the +1 on the original patch. However, please
please please build the code on the target branch before backporting

Eric

On Wed, Jun 2, 2021 at 2:46 PM Ayush Saxena <ay...@gmail.com> wrote:

> For trivial changes, like changes in import or conflicts due to line
> number or other trivial stuff, I don’t think that is required. Unless the
> general  logic isn’t changing, we can go ahead, may be we can do a test run
> before merging, to be on the safer side as and when required. :-)
>
> -Ayush
>
> > On 02-Jun-2021, at 10:13 AM, Wei-Chiu Chuang <we...@apache.org> wrote:
> >
> > I'm curious about the GitHub PR conventions we use today... say I want
> to
> > backport a commit from trunk to branch-3.3, and there's a small code
> > conflict so I push a PR against branch-3.3 using GitHub to go through the
> > precommit check.
> >
> > Do I need explicit approval from another committer to merge the backport
> > PR? (As.a committer, I know I can merge at any time) or can I merge when
> > the precommit comes back okay?
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: hdfs-dev-unsubscribe@hadoop.apache.org
> For additional commands, e-mail: hdfs-dev-help@hadoop.apache.org
>
>

Re: [E] Re: Do I need a +1 to merge a backport PR?

Posted by Eric Badger <eb...@verizonmedia.com.INVALID>.
I'm of a similar opinion to most here. If the backport is clean, I think
it's ok to do it with just the +1 on the original patch. However, please
please please build the code on the target branch before backporting

Eric

On Wed, Jun 2, 2021 at 2:46 PM Ayush Saxena <ay...@gmail.com> wrote:

> For trivial changes, like changes in import or conflicts due to line
> number or other trivial stuff, I don’t think that is required. Unless the
> general  logic isn’t changing, we can go ahead, may be we can do a test run
> before merging, to be on the safer side as and when required. :-)
>
> -Ayush
>
> > On 02-Jun-2021, at 10:13 AM, Wei-Chiu Chuang <we...@apache.org> wrote:
> >
> > I'm curious about the GitHub PR conventions we use today... say I want
> to
> > backport a commit from trunk to branch-3.3, and there's a small code
> > conflict so I push a PR against branch-3.3 using GitHub to go through the
> > precommit check.
> >
> > Do I need explicit approval from another committer to merge the backport
> > PR? (As.a committer, I know I can merge at any time) or can I merge when
> > the precommit comes back okay?
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: hdfs-dev-unsubscribe@hadoop.apache.org
> For additional commands, e-mail: hdfs-dev-help@hadoop.apache.org
>
>

Re: Do I need a +1 to merge a backport PR?

Posted by Ayush Saxena <ay...@gmail.com>.
For trivial changes, like changes in import or conflicts due to line number or other trivial stuff, I don’t think that is required. Unless the general  logic isn’t changing, we can go ahead, may be we can do a test run before merging, to be on the safer side as and when required. :-)

-Ayush

> On 02-Jun-2021, at 10:13 AM, Wei-Chiu Chuang <we...@apache.org> wrote:
> 
> I'm curious about the GitHub PR conventions we use today... say I want to
> backport a commit from trunk to branch-3.3, and there's a small code
> conflict so I push a PR against branch-3.3 using GitHub to go through the
> precommit check.
> 
> Do I need explicit approval from another committer to merge the backport
> PR? (As.a committer, I know I can merge at any time) or can I merge when
> the precommit comes back okay?

---------------------------------------------------------------------
To unsubscribe, e-mail: common-dev-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-dev-help@hadoop.apache.org


Re: Do I need a +1 to merge a backport PR?

Posted by Steve Loughran <st...@cloudera.com.INVALID>.
I think it comes down to "do you think somebody else needs to review it?".

I do like to test before a cherrypick -at least of all the tests which
the patch changed, and for object store stuff a full test run is good due
diligence, but I think at least for me

cherrypick no merge issues: local compile, maybe test, push up
cherrypick import merge issues: the same
merge problems and need to pull in previous work -that's when it gets
complicated.

The last bit of work of mine with major backport was the list-side changes
of HADOOP-13230, where all versions of Hadoop needed to be aware that an
empty directory marker in s3 didn't mean that the path was empty.

That I did as a new JIRA with it's own PRs for each branch, even though it
was just a small subset of the main patch + some of the PathCapabilities
API (HADOOP-17210) for probes (
https://issues.apache.org/jira/browse/HADOOP-17199).

I did all that though the normal build process but once I was happy I did
the merge myself. I could make the case here that (a) the changes were
isolated and (b) they were needed. And I was doing the changes through
gerrit if anybody wanted to see what I was doing.

-Steve



On Wed, 2 Jun 2021 at 06:18, Masatake Iwasaki <iw...@oss.nttdata.co.jp>
wrote:

> I think we usually merge backport without explicit +1 if the conflict is
> trivial,
> same as done on JIRA-only workflow
> (on which we attached patch like HADOOP-xxx-branch-3.3.001.patch for
> precommit check then commit it without explicit +1).
>
> If committers want review since the change is not trivial,
> backporting PR might tend to be ignored without explicit request.
>
> There seems to be no clear statement about approving backports on
> HowToCommit wiki.
> https://cwiki.apache.org/confluence/display/HADOOP2/HowToCommit
>
> On 2021/06/02 13:43, Wei-Chiu Chuang wrote:
> > I'm curious about the GitHub PR conventions we use today... say I want to
> > backport a commit from trunk to branch-3.3, and there's a small code
> > conflict so I push a PR against branch-3.3 using GitHub to go through the
> > precommit check.
> >
> > Do I need explicit approval from another committer to merge the backport
> > PR? (As.a committer, I know I can merge at any time) or can I merge when
> > the precommit comes back okay?
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: hdfs-dev-unsubscribe@hadoop.apache.org
> For additional commands, e-mail: hdfs-dev-help@hadoop.apache.org
>
>

Re: Do I need a +1 to merge a backport PR?

Posted by Steve Loughran <st...@cloudera.com.INVALID>.
I think it comes down to "do you think somebody else needs to review it?".

I do like to test before a cherrypick -at least of all the tests which
the patch changed, and for object store stuff a full test run is good due
diligence, but I think at least for me

cherrypick no merge issues: local compile, maybe test, push up
cherrypick import merge issues: the same
merge problems and need to pull in previous work -that's when it gets
complicated.

The last bit of work of mine with major backport was the list-side changes
of HADOOP-13230, where all versions of Hadoop needed to be aware that an
empty directory marker in s3 didn't mean that the path was empty.

That I did as a new JIRA with it's own PRs for each branch, even though it
was just a small subset of the main patch + some of the PathCapabilities
API (HADOOP-17210) for probes (
https://issues.apache.org/jira/browse/HADOOP-17199).

I did all that though the normal build process but once I was happy I did
the merge myself. I could make the case here that (a) the changes were
isolated and (b) they were needed. And I was doing the changes through
gerrit if anybody wanted to see what I was doing.

-Steve



On Wed, 2 Jun 2021 at 06:18, Masatake Iwasaki <iw...@oss.nttdata.co.jp>
wrote:

> I think we usually merge backport without explicit +1 if the conflict is
> trivial,
> same as done on JIRA-only workflow
> (on which we attached patch like HADOOP-xxx-branch-3.3.001.patch for
> precommit check then commit it without explicit +1).
>
> If committers want review since the change is not trivial,
> backporting PR might tend to be ignored without explicit request.
>
> There seems to be no clear statement about approving backports on
> HowToCommit wiki.
> https://cwiki.apache.org/confluence/display/HADOOP2/HowToCommit
>
> On 2021/06/02 13:43, Wei-Chiu Chuang wrote:
> > I'm curious about the GitHub PR conventions we use today... say I want to
> > backport a commit from trunk to branch-3.3, and there's a small code
> > conflict so I push a PR against branch-3.3 using GitHub to go through the
> > precommit check.
> >
> > Do I need explicit approval from another committer to merge the backport
> > PR? (As.a committer, I know I can merge at any time) or can I merge when
> > the precommit comes back okay?
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: hdfs-dev-unsubscribe@hadoop.apache.org
> For additional commands, e-mail: hdfs-dev-help@hadoop.apache.org
>
>

Re: Do I need a +1 to merge a backport PR?

Posted by Masatake Iwasaki <iw...@oss.nttdata.co.jp>.
I think we usually merge backport without explicit +1 if the conflict is trivial,
same as done on JIRA-only workflow
(on which we attached patch like HADOOP-xxx-branch-3.3.001.patch for
precommit check then commit it without explicit +1).

If committers want review since the change is not trivial,
backporting PR might tend to be ignored without explicit request.

There seems to be no clear statement about approving backports on HowToCommit wiki.
https://cwiki.apache.org/confluence/display/HADOOP2/HowToCommit

On 2021/06/02 13:43, Wei-Chiu Chuang wrote:
> I'm curious about the GitHub PR conventions we use today... say I want to
> backport a commit from trunk to branch-3.3, and there's a small code
> conflict so I push a PR against branch-3.3 using GitHub to go through the
> precommit check.
> 
> Do I need explicit approval from another committer to merge the backport
> PR? (As.a committer, I know I can merge at any time) or can I merge when
> the precommit comes back okay?
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-dev-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-dev-help@hadoop.apache.org


Re: Do I need a +1 to merge a backport PR?

Posted by Masatake Iwasaki <iw...@oss.nttdata.co.jp>.
I think we usually merge backport without explicit +1 if the conflict is trivial,
same as done on JIRA-only workflow
(on which we attached patch like HADOOP-xxx-branch-3.3.001.patch for
precommit check then commit it without explicit +1).

If committers want review since the change is not trivial,
backporting PR might tend to be ignored without explicit request.

There seems to be no clear statement about approving backports on HowToCommit wiki.
https://cwiki.apache.org/confluence/display/HADOOP2/HowToCommit

On 2021/06/02 13:43, Wei-Chiu Chuang wrote:
> I'm curious about the GitHub PR conventions we use today... say I want to
> backport a commit from trunk to branch-3.3, and there's a small code
> conflict so I push a PR against branch-3.3 using GitHub to go through the
> precommit check.
> 
> Do I need explicit approval from another committer to merge the backport
> PR? (As.a committer, I know I can merge at any time) or can I merge when
> the precommit comes back okay?
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: common-dev-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-dev-help@hadoop.apache.org


Re: Do I need a +1 to merge a backport PR?

Posted by "epayne@apache.org" <ep...@apache.org>.
It is okay to go ahead and backport as long as there are no major refactoring necessary.Minor conflict fixes should be fine.-Eric

On Tuesday, June 1, 2021, 11:43:44 PM CDT, Wei-Chiu Chuang <we...@apache.org> wrote: 
 
 I'm curious about the GitHub PR conventions we use today... say I want to
backport a commit from trunk to branch-3.3, and there's a small code
conflict so I push a PR against branch-3.3 using GitHub to go through the
precommit check.

Do I need explicit approval from another committer to merge the backport
PR? (As.a committer, I know I can merge at any time) or can I merge when
the precommit comes back okay?
  

Re: Do I need a +1 to merge a backport PR?

Posted by "epayne@apache.org" <ep...@apache.org>.
It is okay to go ahead and backport as long as there are no major refactoring necessary.Minor conflict fixes should be fine.-Eric

On Tuesday, June 1, 2021, 11:43:44 PM CDT, Wei-Chiu Chuang <we...@apache.org> wrote: 
 
 I'm curious about the GitHub PR conventions we use today... say I want to
backport a commit from trunk to branch-3.3, and there's a small code
conflict so I push a PR against branch-3.3 using GitHub to go through the
precommit check.

Do I need explicit approval from another committer to merge the backport
PR? (As.a committer, I know I can merge at any time) or can I merge when
the precommit comes back okay?
  

Re: Do I need a +1 to merge a backport PR?

Posted by Ayush Saxena <ay...@gmail.com>.
For trivial changes, like changes in import or conflicts due to line number or other trivial stuff, I don’t think that is required. Unless the general  logic isn’t changing, we can go ahead, may be we can do a test run before merging, to be on the safer side as and when required. :-)

-Ayush

> On 02-Jun-2021, at 10:13 AM, Wei-Chiu Chuang <we...@apache.org> wrote:
> 
> I'm curious about the GitHub PR conventions we use today... say I want to
> backport a commit from trunk to branch-3.3, and there's a small code
> conflict so I push a PR against branch-3.3 using GitHub to go through the
> precommit check.
> 
> Do I need explicit approval from another committer to merge the backport
> PR? (As.a committer, I know I can merge at any time) or can I merge when
> the precommit comes back okay?

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-dev-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-dev-help@hadoop.apache.org