You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Jason Gerlowski <ge...@gmail.com> on 2019/09/17 14:12:40 UTC

Github PR Collaboration Question

Hey all,

I've hit a small snag reviewing a few PRs on github recently.  Wanted
to see if anyone has any suggestions for my workflow:

I’ve found myself in the position a few times where I want to add a
few small changes to a contributor’s PR…either to help them with a
piece they haven’t gotten to yet, or to show what I’m suggesting with
a particular review comment, or to clean up little things like
whitespace formatting, etc.  In the patch world, this is easy to do
without requiring review/acking by other contributors…you apply the
latest patch, make your additional changes, and re-upload to jira.
But in Github, you have to juggle branch/PR ownership.  PR’s are often
from personal forks, where others don't have write access there.  So I
can't add to the "main" PR without either (a) asking the contributor
to give me the right karma, or (b) opening my own "secondary" PR
against their "main" PR branch, and asking them to review/merge my
"secondary" PR.

Is there some simpler approach I'm missing?  I love the in-line
comments that Github supports for code-review, and it seems like more
committers are starting to use it.  I'd love to figure out how to make
it work for me.  But two developers collaborating on the same PR seems
like a pretty fundamental use case to be so heavyweight.  This must be
a problem that's solved, right?

Best,

Jason

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


Re: Github PR Collaboration Question

Posted by Jason Gerlowski <ge...@gmail.com>.
Thanks for chiming in guys.  I created a JIRA for this (SOLR-13775).
Haven't really thought about wording or anything yet, if either of you
have any suggestions.  Otherwise I'll hope to get to this over the
weekend.

Jason

On Tue, Sep 17, 2019 at 3:39 PM David Smiley <da...@gmail.com> wrote:
>
> RE the idea of shash-merge from a PR:  that would be cool were it not for CHANGES.txt; ah well.
>
> +1 to "put it in the PR template checklist"
>
> ~ David Smiley
> Apache Lucene/Solr Search Developer
> http://www.linkedin.com/in/davidwsmiley
>
>
> On Tue, Sep 17, 2019 at 2:37 PM Jan Høydahl <ja...@cominvent.com> wrote:
>>
>> I like the idea to put it in the PR template checklist. Or you can ask the contributor to check that box. I once made a PR against another PR branch and it worked but was too many steps.
>>
>> I like the idea of squash-merging from PR branch since (I believe) the commit will then have the credit of the contributor which then shows up in his/her stats on GH which is nice.
>>
>> Jan Høydahl
>>
>> > 17. sep. 2019 kl. 16:17 skrev Jason Gerlowski <ge...@gmail.com>:
>> >
>> > Github does have an option that fork-owners can click when they create
>> > a PR that will give those with karma on the upstream repo the same
>> > karma on their PR branch.  [1] That would solve this problem somewhat.
>> > But it's still up to users to choose that themselves.  Maybe it makes
>> > sense to mention this as an optional checklist item in the PR template
>> > that was recently added.
>> >
>> > Still curious about other approaches though if anyone has suggestions.
>> >
>> > [1] https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork
>> >
>> >> On Tue, Sep 17, 2019 at 10:12 AM Jason Gerlowski <ge...@gmail.com> wrote:
>> >>
>> >> Hey all,
>> >>
>> >> I've hit a small snag reviewing a few PRs on github recently.  Wanted
>> >> to see if anyone has any suggestions for my workflow:
>> >>
>> >> I’ve found myself in the position a few times where I want to add a
>> >> few small changes to a contributor’s PR…either to help them with a
>> >> piece they haven’t gotten to yet, or to show what I’m suggesting with
>> >> a particular review comment, or to clean up little things like
>> >> whitespace formatting, etc.  In the patch world, this is easy to do
>> >> without requiring review/acking by other contributors…you apply the
>> >> latest patch, make your additional changes, and re-upload to jira.
>> >> But in Github, you have to juggle branch/PR ownership.  PR’s are often
>> >> from personal forks, where others don't have write access there.  So I
>> >> can't add to the "main" PR without either (a) asking the contributor
>> >> to give me the right karma, or (b) opening my own "secondary" PR
>> >> against their "main" PR branch, and asking them to review/merge my
>> >> "secondary" PR.
>> >>
>> >> Is there some simpler approach I'm missing?  I love the in-line
>> >> comments that Github supports for code-review, and it seems like more
>> >> committers are starting to use it.  I'd love to figure out how to make
>> >> it work for me.  But two developers collaborating on the same PR seems
>> >> like a pretty fundamental use case to be so heavyweight.  This must be
>> >> a problem that's solved, right?
>> >>
>> >> Best,
>> >>
>> >> Jason
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>> > For additional commands, e-mail: dev-help@lucene.apache.org
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>> For additional commands, e-mail: dev-help@lucene.apache.org
>>

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


Re: Github PR Collaboration Question

Posted by David Smiley <da...@gmail.com>.
RE the idea of shash-merge from a PR:  that would be cool were it not for
CHANGES.txt; ah well.

+1 to "put it in the PR template checklist"

~ David Smiley
Apache Lucene/Solr Search Developer
http://www.linkedin.com/in/davidwsmiley


On Tue, Sep 17, 2019 at 2:37 PM Jan Høydahl <ja...@cominvent.com> wrote:

> I like the idea to put it in the PR template checklist. Or you can ask the
> contributor to check that box. I once made a PR against another PR branch
> and it worked but was too many steps.
>
> I like the idea of squash-merging from PR branch since (I believe) the
> commit will then have the credit of the contributor which then shows up in
> his/her stats on GH which is nice.
>
> Jan Høydahl
>
> > 17. sep. 2019 kl. 16:17 skrev Jason Gerlowski <ge...@gmail.com>:
> >
> > Github does have an option that fork-owners can click when they create
> > a PR that will give those with karma on the upstream repo the same
> > karma on their PR branch.  [1] That would solve this problem somewhat.
> > But it's still up to users to choose that themselves.  Maybe it makes
> > sense to mention this as an optional checklist item in the PR template
> > that was recently added.
> >
> > Still curious about other approaches though if anyone has suggestions.
> >
> > [1]
> https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork
> >
> >> On Tue, Sep 17, 2019 at 10:12 AM Jason Gerlowski <ge...@gmail.com>
> wrote:
> >>
> >> Hey all,
> >>
> >> I've hit a small snag reviewing a few PRs on github recently.  Wanted
> >> to see if anyone has any suggestions for my workflow:
> >>
> >> I’ve found myself in the position a few times where I want to add a
> >> few small changes to a contributor’s PR…either to help them with a
> >> piece they haven’t gotten to yet, or to show what I’m suggesting with
> >> a particular review comment, or to clean up little things like
> >> whitespace formatting, etc.  In the patch world, this is easy to do
> >> without requiring review/acking by other contributors…you apply the
> >> latest patch, make your additional changes, and re-upload to jira.
> >> But in Github, you have to juggle branch/PR ownership.  PR’s are often
> >> from personal forks, where others don't have write access there.  So I
> >> can't add to the "main" PR without either (a) asking the contributor
> >> to give me the right karma, or (b) opening my own "secondary" PR
> >> against their "main" PR branch, and asking them to review/merge my
> >> "secondary" PR.
> >>
> >> Is there some simpler approach I'm missing?  I love the in-line
> >> comments that Github supports for code-review, and it seems like more
> >> committers are starting to use it.  I'd love to figure out how to make
> >> it work for me.  But two developers collaborating on the same PR seems
> >> like a pretty fundamental use case to be so heavyweight.  This must be
> >> a problem that's solved, right?
> >>
> >> Best,
> >>
> >> Jason
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> > For additional commands, e-mail: dev-help@lucene.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
>

Re: Github PR Collaboration Question

Posted by Jan Høydahl <ja...@cominvent.com>.
I like the idea to put it in the PR template checklist. Or you can ask the contributor to check that box. I once made a PR against another PR branch and it worked but was too many steps.

I like the idea of squash-merging from PR branch since (I believe) the commit will then have the credit of the contributor which then shows up in his/her stats on GH which is nice.

Jan Høydahl

> 17. sep. 2019 kl. 16:17 skrev Jason Gerlowski <ge...@gmail.com>:
> 
> Github does have an option that fork-owners can click when they create
> a PR that will give those with karma on the upstream repo the same
> karma on their PR branch.  [1] That would solve this problem somewhat.
> But it's still up to users to choose that themselves.  Maybe it makes
> sense to mention this as an optional checklist item in the PR template
> that was recently added.
> 
> Still curious about other approaches though if anyone has suggestions.
> 
> [1] https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork
> 
>> On Tue, Sep 17, 2019 at 10:12 AM Jason Gerlowski <ge...@gmail.com> wrote:
>> 
>> Hey all,
>> 
>> I've hit a small snag reviewing a few PRs on github recently.  Wanted
>> to see if anyone has any suggestions for my workflow:
>> 
>> I’ve found myself in the position a few times where I want to add a
>> few small changes to a contributor’s PR…either to help them with a
>> piece they haven’t gotten to yet, or to show what I’m suggesting with
>> a particular review comment, or to clean up little things like
>> whitespace formatting, etc.  In the patch world, this is easy to do
>> without requiring review/acking by other contributors…you apply the
>> latest patch, make your additional changes, and re-upload to jira.
>> But in Github, you have to juggle branch/PR ownership.  PR’s are often
>> from personal forks, where others don't have write access there.  So I
>> can't add to the "main" PR without either (a) asking the contributor
>> to give me the right karma, or (b) opening my own "secondary" PR
>> against their "main" PR branch, and asking them to review/merge my
>> "secondary" PR.
>> 
>> Is there some simpler approach I'm missing?  I love the in-line
>> comments that Github supports for code-review, and it seems like more
>> committers are starting to use it.  I'd love to figure out how to make
>> it work for me.  But two developers collaborating on the same PR seems
>> like a pretty fundamental use case to be so heavyweight.  This must be
>> a problem that's solved, right?
>> 
>> Best,
>> 
>> Jason
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
> 

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


Re: Github PR Collaboration Question

Posted by Jason Gerlowski <ge...@gmail.com>.
Github does have an option that fork-owners can click when they create
a PR that will give those with karma on the upstream repo the same
karma on their PR branch.  [1] That would solve this problem somewhat.
But it's still up to users to choose that themselves.  Maybe it makes
sense to mention this as an optional checklist item in the PR template
that was recently added.

Still curious about other approaches though if anyone has suggestions.

[1] https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork

On Tue, Sep 17, 2019 at 10:12 AM Jason Gerlowski <ge...@gmail.com> wrote:
>
> Hey all,
>
> I've hit a small snag reviewing a few PRs on github recently.  Wanted
> to see if anyone has any suggestions for my workflow:
>
> I’ve found myself in the position a few times where I want to add a
> few small changes to a contributor’s PR…either to help them with a
> piece they haven’t gotten to yet, or to show what I’m suggesting with
> a particular review comment, or to clean up little things like
> whitespace formatting, etc.  In the patch world, this is easy to do
> without requiring review/acking by other contributors…you apply the
> latest patch, make your additional changes, and re-upload to jira.
> But in Github, you have to juggle branch/PR ownership.  PR’s are often
> from personal forks, where others don't have write access there.  So I
> can't add to the "main" PR without either (a) asking the contributor
> to give me the right karma, or (b) opening my own "secondary" PR
> against their "main" PR branch, and asking them to review/merge my
> "secondary" PR.
>
> Is there some simpler approach I'm missing?  I love the in-line
> comments that Github supports for code-review, and it seems like more
> committers are starting to use it.  I'd love to figure out how to make
> it work for me.  But two developers collaborating on the same PR seems
> like a pretty fundamental use case to be so heavyweight.  This must be
> a problem that's solved, right?
>
> Best,
>
> Jason

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