You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Maarten Mulders <mt...@apache.org> on 2020/07/13 18:20:43 UTC

Short guide "Working with Github pull requests"

Hi all,

The other day I was working on a pull request offered through Github. I
couldn't find instructions on how to accept and incorporate it. Based on
my own experiences with submitting patches to Maven through Github I
came up with a short how-to.

I have written it down in a short guide "Working with Github pull
requests" [1] which I intent to publish on the Apache Maven site, under
"Maven Developer Centre / Committers". I would appreciate if some of you
could take a look and see if the guide makes sense.

Thanks,

Maarten

[1]
https://gitbox.apache.org/repos/asf?p=maven-site.git;a=shortlog;h=refs/heads/add-github-pull-request-guide

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


Re: Short guide "Working with Github pull requests"

Posted by Enrico Olivelli <eo...@gmail.com>.
Il Mar 14 Lug 2020, 13:09 Elliotte Rusty Harold <el...@ibiblio.org> ha
scritto:

> On Tue, Jul 14, 2020 at 2:25 AM Olivier Lamy <ol...@apache.org> wrote:
> >
>
> > ***DO NOT CREATE PULL REQUEST FOR RIDICULOUS COMMIT WHICH DO NOT NEED
> REVIEW***
>
> IMHO all PRs need review for the same reason everything needs a test.
>

I see your pain. Having reviews by fellow committers and from the community
helps a lot.
I felt this way the first times I got write permission to Maven
repositories.

But we have many repositories and those simple pull requests really sound
as noise.

Every commit you push to a shared repository triggers an email and every
other committer (and anyone who is subscribe to the ml) sees your commit
and can chime in in case there is something wrong.

So feel free to commit simple stuff as far as you stay into our conventions
and integration tests pass.
We are all here and see the flow of commits.
Feel free to ask for review, in that case I prefer a PR.

I hope that helps
Enrico


From experience I know I can't tell the difference between PRs that
> are too simple to fail and PRs that only look that way.
>
> > it's very noisy and useless notifications (especially because we get
> from both gitbox and github....)
>
> Not sure why you're seeing all these notifications. I don't see them.
>

Probably because you are the author


However if both gitbox and github are sending notices for the same PR,
> there's a really easy fix. Simply disable one of the two from sending
> any notifications.
>
> --
> Elliotte Rusty Harold
> elharo@ibiblio.org
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Re: Short guide "Working with Github pull requests"

Posted by Elliotte Rusty Harold <el...@ibiblio.org>.
On Tue, Jul 14, 2020 at 2:25 AM Olivier Lamy <ol...@apache.org> wrote:
>

> ***DO NOT CREATE PULL REQUEST FOR RIDICULOUS COMMIT WHICH DO NOT NEED REVIEW***

IMHO all PRs need review for the same reason everything needs a test.
From experience I know I can't tell the difference between PRs that
are too simple to fail and PRs that only look that way.

> it's very noisy and useless notifications (especially because we get from both gitbox and github....)

Not sure why you're seeing all these notifications. I don't see them.
However if both gitbox and github are sending notices for the same PR,
there's a really easy fix. Simply disable one of the two from sending
any notifications.

-- 
Elliotte Rusty Harold
elharo@ibiblio.org

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


Re: Short guide "Working with Github pull requests"

Posted by Olivier Lamy <ol...@apache.org>.
On Tue, 14 Jul 2020 at 16:37, Maarten Mulders <mt...@apache.org> wrote:

> Good part is, even though I followed the docs I couldn't get it to work
> for even one repo (maven-site). It still has the "Squash & merge" button.
>
> The guide I was proposing was for working with pull requests submitted
> by contributors, not for creating them. So I wasn't planning to add any
> words on when to create a pull request or not.
>
> I do agree that there's a lot of email generated. Sometimes even
> multiple emails with the same content in them. I think we should solve
> that by sending less notifications, not by creating less pull requests.
>

good point :) but remember we are commit then review mode
So for trivial change (such typo or some very ezy change) you don't need to.
Don't be scared to push without review, perso I will never blame someone
because he is doing something :)
If you're not confident with your changes go ahead and create PR
(especially if you want CI build) but please don't turn this into some
bureaucratic stuff.....

(welcome in the jungle with everyone having different opinion :) )

There must be a way, somewhere, to disable either the Github ones or the
> Gitbox ones.
>
> Maarten
>
> On 14/07/2020 08:25, Olivier Lamy wrote:
> > Well personally I'm happy to use it for myself...
> > so please don;t remove it.
> > One of the most important point of the guide could be
> > ***DO NOT CREATE PULL REQUEST FOR RIDICULOUS COMMIT WHICH DO NOT NEED
> > REVIEW***
> >
> > it's very noisy and useless notifications (especially because we get
> > from both gitbox and github....)
> >
> >
> > On Tue, 14 Jul 2020 at 16:18, Maarten Mulders <mt...@apache.org>
> > wrote:
> >
> >> Good point. I'll start by disabling the "Squash & merge" button using
> >> asf.yaml for the repositories I have cloned locally.
> >>
> >> Maarten
> >>
> >> On 13/07/2020 22:03, Elliotte Rusty Harold wrote:
> >>>>> So then the conclusion is, we can use Github for accepting & merging
> >>>>> pull requests, but shouldn't use "Squash & merge"?
> >>>
> >>> Buttons that shouldn't be used can be turned off by project owners in
> >>> the repo settings. There's no need to bother devs with remembering
> >>> which buttons should and should not be pressed (especially since
> >>> you'll get different answers to this question depending on who you
> >>> ask.)
> >>>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >> For additional commands, e-mail: dev-help@maven.apache.org
> >>
> >>
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

-- 
Olivier Lamy
http://twitter.com/olamy | http://linkedin.com/in/olamy

Re: Short guide "Working with Github pull requests"

Posted by Maarten Mulders <mt...@apache.org>.
Good part is, even though I followed the docs I couldn't get it to work
for even one repo (maven-site). It still has the "Squash & merge" button.

The guide I was proposing was for working with pull requests submitted
by contributors, not for creating them. So I wasn't planning to add any
words on when to create a pull request or not.

I do agree that there's a lot of email generated. Sometimes even
multiple emails with the same content in them. I think we should solve
that by sending less notifications, not by creating less pull requests.
There must be a way, somewhere, to disable either the Github ones or the
Gitbox ones.

Maarten

On 14/07/2020 08:25, Olivier Lamy wrote:
> Well personally I'm happy to use it for myself...
> so please don;t remove it.
> One of the most important point of the guide could be
> ***DO NOT CREATE PULL REQUEST FOR RIDICULOUS COMMIT WHICH DO NOT NEED
> REVIEW***
> 
> it's very noisy and useless notifications (especially because we get
> from both gitbox and github....)
> 
> 
> On Tue, 14 Jul 2020 at 16:18, Maarten Mulders <mt...@apache.org>
> wrote:
> 
>> Good point. I'll start by disabling the "Squash & merge" button using
>> asf.yaml for the repositories I have cloned locally.
>>
>> Maarten
>>
>> On 13/07/2020 22:03, Elliotte Rusty Harold wrote:
>>>>> So then the conclusion is, we can use Github for accepting & merging
>>>>> pull requests, but shouldn't use "Squash & merge"?
>>>
>>> Buttons that shouldn't be used can be turned off by project owners in
>>> the repo settings. There's no need to bother devs with remembering
>>> which buttons should and should not be pressed (especially since
>>> you'll get different answers to this question depending on who you
>>> ask.)
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>>
> 

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


Re: Short guide "Working with Github pull requests"

Posted by Olivier Lamy <ol...@apache.org>.
Well personally I'm happy to use it for myself...
so please don;t remove it.
One of the most important point of the guide could be
***DO NOT CREATE PULL REQUEST FOR RIDICULOUS COMMIT WHICH DO NOT NEED
REVIEW***

it's very noisy and useless notifications (especially because we get
from both gitbox and github....)


On Tue, 14 Jul 2020 at 16:18, Maarten Mulders <m....@xs4all.nl>
wrote:

> Good point. I'll start by disabling the "Squash & merge" button using
> asf.yaml for the repositories I have cloned locally.
>
> Maarten
>
> On 13/07/2020 22:03, Elliotte Rusty Harold wrote:
> >>> So then the conclusion is, we can use Github for accepting & merging
> >>> pull requests, but shouldn't use "Squash & merge"?
> >
> > Buttons that shouldn't be used can be turned off by project owners in
> > the repo settings. There's no need to bother devs with remembering
> > which buttons should and should not be pressed (especially since
> > you'll get different answers to this question depending on who you
> > ask.)
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

-- 
Olivier Lamy
http://twitter.com/olamy | http://linkedin.com/in/olamy

Re: Short guide "Working with Github pull requests"

Posted by Maarten Mulders <m....@xs4all.nl>.
Good point. I'll start by disabling the "Squash & merge" button using
asf.yaml for the repositories I have cloned locally.

Maarten

On 13/07/2020 22:03, Elliotte Rusty Harold wrote:
>>> So then the conclusion is, we can use Github for accepting & merging
>>> pull requests, but shouldn't use "Squash & merge"?
> 
> Buttons that shouldn't be used can be turned off by project owners in
> the repo settings. There's no need to bother devs with remembering
> which buttons should and should not be pressed (especially since
> you'll get different answers to this question depending on who you
> ask.)
> 

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


Re: Short guide "Working with Github pull requests"

Posted by Elliotte Rusty Harold <el...@ibiblio.org>.
> > So then the conclusion is, we can use Github for accepting & merging
> > pull requests, but shouldn't use "Squash & merge"?

Buttons that shouldn't be used can be turned off by project owners in
the repo settings. There's no need to bother devs with remembering
which buttons should and should not be pressed (especially since
you'll get different answers to this question depending on who you
ask.)

-- 
Elliotte Rusty Harold
elharo@ibiblio.org

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


Re: Short guide "Working with Github pull requests"

Posted by Enrico Olivelli <eo...@gmail.com>.
Il Lun 13 Lug 2020, 21:03 Maarten Mulders <mt...@apache.org> ha
scritto:

> So, just to make sure: the sync is two-way?


Yes

What happens on Gitbox is
> synced to Github and vice versa? I thought it was only from Gitbox to
> Github, but never the other way around...
>

No. We should ask infra to know how this magic happens

>
> I do agree that Githubs UI is a bit more sophisticated for dealing with
> merge requests. I vaguely recall an earlier discussion on this list
> about the use of Githubs "Squash and merge" button. I thought the
> outcome of that discussion was that we shouldn't use it, because the
> author in the commit log would show as "Github", or something like that.
>
> So then the conclusion is, we can use Github for accepting & merging
> pull requests, but shouldn't use "Squash & merge"?
>

Yes, do not use the button

Enrico



> Maarten
>
> On July 13th, 2020 at 20:28, Elliotte Rusty Harold wrote:
> > Much work happens directly on Github and syncs the other direction to
> > Gitbox. That workflow is likely more familiar to most developers these
> > days, and the Github UI is significantly advanced beyond Gitbox.
> >
> > On Mon, Jul 13, 2020 at 2:20 PM Maarten Mulders <mt...@apache.org>
> wrote:
> >>
> >> Hi all,
> >>
> >> The other day I was working on a pull request offered through Github. I
> >> couldn't find instructions on how to accept and incorporate it. Based on
> >> my own experiences with submitting patches to Maven through Github I
> >> came up with a short how-to.
> >>
> >> I have written it down in a short guide "Working with Github pull
> >> requests" [1] which I intent to publish on the Apache Maven site, under
> >> "Maven Developer Centre / Committers". I would appreciate if some of you
> >> could take a look and see if the guide makes sense.
> >>
> >> Thanks,
> >>
> >> Maarten
> >>
> >> [1]
> >>
> https://gitbox.apache.org/repos/asf?p=maven-site.git;a=shortlog;h=refs/heads/add-github-pull-request-guide
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >> For additional commands, e-mail: dev-help@maven.apache.org
> >>
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Re: Short guide "Working with Github pull requests"

Posted by Maarten Mulders <mt...@apache.org>.
So, just to make sure: the sync is two-way? What happens on Gitbox is
synced to Github and vice versa? I thought it was only from Gitbox to
Github, but never the other way around...

I do agree that Githubs UI is a bit more sophisticated for dealing with
merge requests. I vaguely recall an earlier discussion on this list
about the use of Githubs "Squash and merge" button. I thought the
outcome of that discussion was that we shouldn't use it, because the
author in the commit log would show as "Github", or something like that.

So then the conclusion is, we can use Github for accepting & merging
pull requests, but shouldn't use "Squash & merge"?

Maarten

On July 13th, 2020 at 20:28, Elliotte Rusty Harold wrote:
> Much work happens directly on Github and syncs the other direction to
> Gitbox. That workflow is likely more familiar to most developers these
> days, and the Github UI is significantly advanced beyond Gitbox.
> 
> On Mon, Jul 13, 2020 at 2:20 PM Maarten Mulders <mt...@apache.org> wrote:
>>
>> Hi all,
>>
>> The other day I was working on a pull request offered through Github. I
>> couldn't find instructions on how to accept and incorporate it. Based on
>> my own experiences with submitting patches to Maven through Github I
>> came up with a short how-to.
>>
>> I have written it down in a short guide "Working with Github pull
>> requests" [1] which I intent to publish on the Apache Maven site, under
>> "Maven Developer Centre / Committers". I would appreciate if some of you
>> could take a look and see if the guide makes sense.
>>
>> Thanks,
>>
>> Maarten
>>
>> [1]
>> https://gitbox.apache.org/repos/asf?p=maven-site.git;a=shortlog;h=refs/heads/add-github-pull-request-guide
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
> 
> 

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


Re: Short guide "Working with Github pull requests"

Posted by Elliotte Rusty Harold <el...@ibiblio.org>.
Much work happens directly on Github and syncs the other direction to
Gitbox. That workflow is likely more familiar to most developers these
days, and the Github UI is significantly advanced beyond Gitbox.

On Mon, Jul 13, 2020 at 2:20 PM Maarten Mulders <mt...@apache.org> wrote:
>
> Hi all,
>
> The other day I was working on a pull request offered through Github. I
> couldn't find instructions on how to accept and incorporate it. Based on
> my own experiences with submitting patches to Maven through Github I
> came up with a short how-to.
>
> I have written it down in a short guide "Working with Github pull
> requests" [1] which I intent to publish on the Apache Maven site, under
> "Maven Developer Centre / Committers". I would appreciate if some of you
> could take a look and see if the guide makes sense.
>
> Thanks,
>
> Maarten
>
> [1]
> https://gitbox.apache.org/repos/asf?p=maven-site.git;a=shortlog;h=refs/heads/add-github-pull-request-guide
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>


-- 
Elliotte Rusty Harold
elharo@ibiblio.org

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