You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cassandra.apache.org by "jay.zhuang@yahoo.com.INVALID" <ja...@yahoo.com.INVALID> on 2018/12/12 19:20:48 UTC
Revisit the proposal to use github PR
It was discussed 1 year's ago: https://www.mail-archive.com/dev@cassandra.apache.org/msg11810.html
As all Apache projects are moving to gitbox: https://reference.apache.org/committer/github, should we revisit that and change our review/commit process to use github PR?A good example is Spark:"Changes to Spark source code are proposed, reviewed and committed via Github pull requests" (https://spark.apache.org/contributing.html).
/jay
Re: Revisit the proposal to use github PR
Posted by Jason Brown <ja...@gmail.com>.
I'm with Sylvain and Aleksey. Our multi-branch strategy does not work well
(if at all) with Github PR-style workflows (and, yes, I and most c*
maintainers have played this game). The cassandra-dtests repo is different
as we don't really branch there, so might be fine for Github PRs - except
for the merge commit noise, and that we prefer squashed patches for commit.
We could perhaps be a bit more explicit about "how to contribute" in the
docs [1], wrt branches as well as Github PRs (that we don't do them), but
it's mostly not unreasonable.
Thus, I'm in favor of the current process.
[1] http://cassandra.apache.org/doc/latest/development/patches.html
On Thu, Dec 13, 2018 at 9:20 AM Aleksey Yeschenko <al...@yeschenko.com>
wrote:
> There are some nice benefits to GH PRs, one of them is that we could
> eventually set up CircleCI hooks that would explicitly prevent commits that
> don’t pass the tests.
>
> But handling multiple branches would indeed be annoying. Would have to
> either submit 1 PR per branch - which is both tedious and non-atomic - or
> do a mixed approach, with a PR for the oldest branch, then a manual merge
> upwards. The latter would be kinda meh, especially when commits for
> different branches diverge.
>
> For me personally, the current setup works quite well, and I mostly share
> Sylvain’s opinion above, for the same reasons listed.
>
> —
> AY
>
> > On 13 Dec 2018, at 08:15, Sylvain Lebresne <le...@gmail.com> wrote:
> >
> > Fwiw, I personally find it very useful to have all discussion, review
> > comments included, in the same place (namely JIRA, since for better or
> > worse, that's what we use for tracking tickets). Typically, that means
> > everything gets consistently pushed to the commits@ mailing list,
> which I
> > find extremely convenient to keep track of things. I also have a theory
> > that the inline-comments type of review github PR give you is very
> > convenient for nitpicks, shallow or spur-of-the-moment comments, but
> > doesn't help that much for deeper reviews, and that it thus to favor the
> > former kind of review.
> >
> > Additionally, and to Benedict's point, I happen to have first hand
> > experience with a PR-based process for a multi-branch workflow very
> similar
> > to the one of this project, and suffice to say that I hate it with a
> > passion.
> >
> > Anyway, very much personal opinion here.
> > --
> > Sylvain
> >
> >
> > On Thu, Dec 13, 2018 at 2:13 AM dinesh.joshi@yahoo.com.INVALID
> > <di...@yahoo.com.invalid> wrote:
> >
> >> I've been already using github PRs for some time now. Once you specify
> the
> >> ticket number, the comments and discussion are persisted in Apache Jira
> as
> >> work log so it can be audited if desired. However, committers usually
> >> squash and commit the changes once the PR is approved. We don't use the
> >> merge feature in github. I don't believe github we can merge the commit
> >> into multiple branches through the UI. We would need to merge it into
> one
> >> branch and then manually merge that commit into other branches. The big
> >> upside of using github PRs is that it makes collaborating a lot easier.
> >> Downside is that it makes it very difficult to follow along the
> progress in
> >> Apache Jira. The messages that github posts back include huge diffs and
> are
> >> aweful.
> >> Dinesh
> >>
> >> On Thursday, December 13, 2018, 1:10:12 AM GMT+5:30, Benedict Elliott
> >> Smith <be...@apache.org> wrote:
> >>
> >> Perhaps somebody could summarise the tradeoffs? I’m a little concerned
> >> about how it would work for our multi-branch workflow. Would we open
> >> multiple PRs?
> >>
> >> Could we easily link with external CircleCI?
> >>
> >> It occurs to me, in JIRA proposal mode, that an extra required field
> for a
> >> permalink to GitHub for the patch would save a lot of time I spend
> hunting
> >> for a branch in the comments.
> >>
> >>
> >>
> >>
> >>> On 12 Dec 2018, at 19:20, jay.zhuang@yahoo.com.INVALID wrote:
> >>>
> >>> It was discussed 1 year's ago:
> >> https://www.mail-archive.com/dev@cassandra.apache.org/msg11810.html
> >>> As all Apache projects are moving to gitbox:
> >> https://reference.apache.org/committer/github, should we revisit that
> and
> >> change our review/commit process to use github PR?A good example is
> >> Spark:"Changes to Spark source code are proposed, reviewed and committed
> >> via Github pull requests" (https://spark.apache.org/contributing.html).
> >>> /jay
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
> >> For additional commands, e-mail: dev-help@cassandra.apache.org
> >>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
> For additional commands, e-mail: dev-help@cassandra.apache.org
>
>
Re: Revisit the proposal to use github PR
Posted by Ariel Weisberg <ar...@weisberg.ws>.
Hi,
Sorry I missed that point. I agree github PRs are not useful for merging.
What I do is force push the feature/bug fix branches (which is fine, github remembers the old versions in the PR) with everything updated and ready to merge, and then push those branches from my local repo to the apache repo with --atomic.
Ariel
On Thu, Dec 13, 2018, at 1:00 PM, Jason Brown wrote:
> To clarify my position: Github PRs are great for *reviewing* code, and the
> commentary is much easier to follow imo. But for *merging* code, esp into
> our multi-branch strategy, PRs don't fit well, unless there's some
> technique I and perhaps others are unaware of.
>
> On Thu, Dec 13, 2018 at 9:47 AM Ariel Weisberg <ar...@weisberg.ws> wrote:
>
> > Hi,
> >
> > I'm not clear on what github makes worse. It preserves more history then
> > the JIRA approach. When people invitably force push their branches you
> > can't tell from the link to a branch on JIRA. Github preserves the comments
> > and force push history so you know what version of the code each comment
> > applied to. Github also tracks when requests for changes are acknowledged
> > and resolved. I have had to make the same change request many times and
> > keep track independently whether it was resolved. This has also resulting
> > in mistakes getting merged when I missed a comment that was ignored.
> >
> > Now that github can CC JIRA that also CCs to commits@. It's better then
> > JIRA comments because each comment includes a small diff of the code the
> > comment applies to. To do that in JIRA I have to manually link to the code
> > the PR and most people don't do that for every comment so some of them are
> > inscrutable after the fact. Also manually created links sometimes refer to
> > references that disappear or get force pushed. It's a bit tricky to get
> > right.
> >
> > To me arguing against leveraging a better code review workflow (whether
> > github or some other tool) is like arguing against using source control
> > tools. Sure the filesystem and home grown scripts can be used to work
> > around lack of source control, but why would you?
> >
> > I see two complaints so far. One is that github PRs encourage nitpicking.
> > I don't see tool based solution to that off the cuff. Another is that
> > github doesn't by default CC JIRA. Maybe we can just refuse to accept
> > improperly formatted PRs and look into auto rejecting ones that don't refer
> > to a ticket?
> >
> > Ariel
> >
> > On Thu, Dec 13, 2018, at 12:20 PM, Aleksey Yeschenko wrote:
> > > There are some nice benefits to GH PRs, one of them is that we could
> > > eventually set up CircleCI hooks that would explicitly prevent commits
> > > that don’t pass the tests.
> > >
> > > But handling multiple branches would indeed be annoying. Would have to
> > > either submit 1 PR per branch - which is both tedious and non-atomic -
> > > or do a mixed approach, with a PR for the oldest branch, then a manual
> > > merge upwards. The latter would be kinda meh, especially when commits
> > > for different branches diverge.
> > >
> > > For me personally, the current setup works quite well, and I mostly
> > > share Sylvain’s opinion above, for the same reasons listed.
> > >
> > > —
> > > AY
> > >
> > > > On 13 Dec 2018, at 08:15, Sylvain Lebresne <le...@gmail.com> wrote:
> > > >
> > > > Fwiw, I personally find it very useful to have all discussion, review
> > > > comments included, in the same place (namely JIRA, since for better or
> > > > worse, that's what we use for tracking tickets). Typically, that means
> > > > everything gets consistently pushed to the commits@ mailing list,
> > which I
> > > > find extremely convenient to keep track of things. I also have a theory
> > > > that the inline-comments type of review github PR give you is very
> > > > convenient for nitpicks, shallow or spur-of-the-moment comments, but
> > > > doesn't help that much for deeper reviews, and that it thus to favor
> > the
> > > > former kind of review.
> > > >
> > > > Additionally, and to Benedict's point, I happen to have first hand
> > > > experience with a PR-based process for a multi-branch workflow very
> > similar
> > > > to the one of this project, and suffice to say that I hate it with a
> > > > passion.
> > > >
> > > > Anyway, very much personal opinion here.
> > > > --
> > > > Sylvain
> > > >
> > > >
> > > > On Thu, Dec 13, 2018 at 2:13 AM dinesh.joshi@yahoo.com.INVALID
> > > > <di...@yahoo.com.invalid> wrote:
> > > >
> > > >> I've been already using github PRs for some time now. Once you
> > specify the
> > > >> ticket number, the comments and discussion are persisted in Apache
> > Jira as
> > > >> work log so it can be audited if desired. However, committers usually
> > > >> squash and commit the changes once the PR is approved. We don't use
> > the
> > > >> merge feature in github. I don't believe github we can merge the
> > commit
> > > >> into multiple branches through the UI. We would need to merge it into
> > one
> > > >> branch and then manually merge that commit into other branches. The
> > big
> > > >> upside of using github PRs is that it makes collaborating a lot
> > easier.
> > > >> Downside is that it makes it very difficult to follow along the
> > progress in
> > > >> Apache Jira. The messages that github posts back include huge diffs
> > and are
> > > >> aweful.
> > > >> Dinesh
> > > >>
> > > >> On Thursday, December 13, 2018, 1:10:12 AM GMT+5:30, Benedict
> > Elliott
> > > >> Smith <be...@apache.org> wrote:
> > > >>
> > > >> Perhaps somebody could summarise the tradeoffs? I’m a little
> > concerned
> > > >> about how it would work for our multi-branch workflow. Would we open
> > > >> multiple PRs?
> > > >>
> > > >> Could we easily link with external CircleCI?
> > > >>
> > > >> It occurs to me, in JIRA proposal mode, that an extra required field
> > for a
> > > >> permalink to GitHub for the patch would save a lot of time I spend
> > hunting
> > > >> for a branch in the comments.
> > > >>
> > > >>
> > > >>
> > > >>
> > > >>> On 12 Dec 2018, at 19:20, jay.zhuang@yahoo.com.INVALID wrote:
> > > >>>
> > > >>> It was discussed 1 year's ago:
> > > >> https://www.mail-archive.com/dev@cassandra.apache.org/msg11810.html
> > > >>> As all Apache projects are moving to gitbox:
> > > >> https://reference.apache.org/committer/github, should we revisit
> > that and
> > > >> change our review/commit process to use github PR?A good example is
> > > >> Spark:"Changes to Spark source code are proposed, reviewed and
> > committed
> > > >> via Github pull requests" (https://spark.apache.org/contributing.html
> > ).
> > > >>> /jay
> > > >>
> > > >>
> > > >> ---------------------------------------------------------------------
> > > >> To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
> > > >> For additional commands, e-mail: dev-help@cassandra.apache.org
> > > >>
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
> > > For additional commands, e-mail: dev-help@cassandra.apache.org
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
> > For additional commands, e-mail: dev-help@cassandra.apache.org
> >
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
For additional commands, e-mail: dev-help@cassandra.apache.org
Re: Revisit the proposal to use github PR
Posted by Benedict Elliott Smith <be...@apache.org>.
So, without hopefully getting too dragged into this, I’m honestly not convinced that GitHub is so phenomenal for review. Though perhaps I just suck at the UX.
- Missed notifications, to both me and seemingly Jira. Jira sometimes fails my personal email too, but commits@ can be relied upon.
- Replication from GitHub PRs is super unpleasant to read (so I don’t).
- Complex patches are noisy - impossible to easily signal/emphasise important commentary; gets lost in the maelstrom of nitpicks
- Stale comments. Force-pushes lose their context. Perhaps you can expand them? At the very least, they are hidden by default, meaning probably the most important commentary that required major changes is least accessible.
I have personally found a great balance to be using GitHub for nitpicks or questions, because in their context they’re a lot easier for both parties, and they’re also less critical. I encourage questions to be resolved by actually commenting the code, so that future readers don’t have to look in GitHub or anywhere else for the answer.
Complex feedback potentially requiring a rethink of the approach, or a refactor, etc, I prefer to leave in Jira. We have a lot of practice having complex discussions here, and are fairly good at keeping the discussion focused, to de/emphasise un/important points.
That said, I would be totally fine with using GitHub PRs, if we keep important discussions to Jira.
Basically, +1 Sylvain’s view, or thereabouts.
> On 13 Dec 2018, at 18:00, Jason Brown <ja...@gmail.com> wrote:
>
> To clarify my position: Github PRs are great for *reviewing* code, and the
> commentary is much easier to follow imo. But for *merging* code, esp into
> our multi-branch strategy, PRs don't fit well, unless there's some
> technique I and perhaps others are unaware of.
>
> On Thu, Dec 13, 2018 at 9:47 AM Ariel Weisberg <ar...@weisberg.ws> wrote:
>
>> Hi,
>>
>> I'm not clear on what github makes worse. It preserves more history then
>> the JIRA approach. When people invitably force push their branches you
>> can't tell from the link to a branch on JIRA. Github preserves the comments
>> and force push history so you know what version of the code each comment
>> applied to. Github also tracks when requests for changes are acknowledged
>> and resolved. I have had to make the same change request many times and
>> keep track independently whether it was resolved. This has also resulting
>> in mistakes getting merged when I missed a comment that was ignored.
>>
>> Now that github can CC JIRA that also CCs to commits@. It's better then
>> JIRA comments because each comment includes a small diff of the code the
>> comment applies to. To do that in JIRA I have to manually link to the code
>> the PR and most people don't do that for every comment so some of them are
>> inscrutable after the fact. Also manually created links sometimes refer to
>> references that disappear or get force pushed. It's a bit tricky to get
>> right.
>>
>> To me arguing against leveraging a better code review workflow (whether
>> github or some other tool) is like arguing against using source control
>> tools. Sure the filesystem and home grown scripts can be used to work
>> around lack of source control, but why would you?
>>
>> I see two complaints so far. One is that github PRs encourage nitpicking.
>> I don't see tool based solution to that off the cuff. Another is that
>> github doesn't by default CC JIRA. Maybe we can just refuse to accept
>> improperly formatted PRs and look into auto rejecting ones that don't refer
>> to a ticket?
>>
>> Ariel
>>
>> On Thu, Dec 13, 2018, at 12:20 PM, Aleksey Yeschenko wrote:
>>> There are some nice benefits to GH PRs, one of them is that we could
>>> eventually set up CircleCI hooks that would explicitly prevent commits
>>> that don’t pass the tests.
>>>
>>> But handling multiple branches would indeed be annoying. Would have to
>>> either submit 1 PR per branch - which is both tedious and non-atomic -
>>> or do a mixed approach, with a PR for the oldest branch, then a manual
>>> merge upwards. The latter would be kinda meh, especially when commits
>>> for different branches diverge.
>>>
>>> For me personally, the current setup works quite well, and I mostly
>>> share Sylvain’s opinion above, for the same reasons listed.
>>>
>>> —
>>> AY
>>>
>>>> On 13 Dec 2018, at 08:15, Sylvain Lebresne <le...@gmail.com> wrote:
>>>>
>>>> Fwiw, I personally find it very useful to have all discussion, review
>>>> comments included, in the same place (namely JIRA, since for better or
>>>> worse, that's what we use for tracking tickets). Typically, that means
>>>> everything gets consistently pushed to the commits@ mailing list,
>> which I
>>>> find extremely convenient to keep track of things. I also have a theory
>>>> that the inline-comments type of review github PR give you is very
>>>> convenient for nitpicks, shallow or spur-of-the-moment comments, but
>>>> doesn't help that much for deeper reviews, and that it thus to favor
>> the
>>>> former kind of review.
>>>>
>>>> Additionally, and to Benedict's point, I happen to have first hand
>>>> experience with a PR-based process for a multi-branch workflow very
>> similar
>>>> to the one of this project, and suffice to say that I hate it with a
>>>> passion.
>>>>
>>>> Anyway, very much personal opinion here.
>>>> --
>>>> Sylvain
>>>>
>>>>
>>>> On Thu, Dec 13, 2018 at 2:13 AM dinesh.joshi@yahoo.com.INVALID
>>>> <di...@yahoo.com.invalid> wrote:
>>>>
>>>>> I've been already using github PRs for some time now. Once you
>> specify the
>>>>> ticket number, the comments and discussion are persisted in Apache
>> Jira as
>>>>> work log so it can be audited if desired. However, committers usually
>>>>> squash and commit the changes once the PR is approved. We don't use
>> the
>>>>> merge feature in github. I don't believe github we can merge the
>> commit
>>>>> into multiple branches through the UI. We would need to merge it into
>> one
>>>>> branch and then manually merge that commit into other branches. The
>> big
>>>>> upside of using github PRs is that it makes collaborating a lot
>> easier.
>>>>> Downside is that it makes it very difficult to follow along the
>> progress in
>>>>> Apache Jira. The messages that github posts back include huge diffs
>> and are
>>>>> aweful.
>>>>> Dinesh
>>>>>
>>>>> On Thursday, December 13, 2018, 1:10:12 AM GMT+5:30, Benedict
>> Elliott
>>>>> Smith <be...@apache.org> wrote:
>>>>>
>>>>> Perhaps somebody could summarise the tradeoffs? I’m a little
>> concerned
>>>>> about how it would work for our multi-branch workflow. Would we open
>>>>> multiple PRs?
>>>>>
>>>>> Could we easily link with external CircleCI?
>>>>>
>>>>> It occurs to me, in JIRA proposal mode, that an extra required field
>> for a
>>>>> permalink to GitHub for the patch would save a lot of time I spend
>> hunting
>>>>> for a branch in the comments.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> On 12 Dec 2018, at 19:20, jay.zhuang@yahoo.com.INVALID wrote:
>>>>>>
>>>>>> It was discussed 1 year's ago:
>>>>> https://www.mail-archive.com/dev@cassandra.apache.org/msg11810.html
>>>>>> As all Apache projects are moving to gitbox:
>>>>> https://reference.apache.org/committer/github, should we revisit
>> that and
>>>>> change our review/commit process to use github PR?A good example is
>>>>> Spark:"Changes to Spark source code are proposed, reviewed and
>> committed
>>>>> via Github pull requests" (https://spark.apache.org/contributing.html
>> ).
>>>>>> /jay
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
>>>>> For additional commands, e-mail: dev-help@cassandra.apache.org
>>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
>>> For additional commands, e-mail: dev-help@cassandra.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
>> For additional commands, e-mail: dev-help@cassandra.apache.org
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
For additional commands, e-mail: dev-help@cassandra.apache.org
Re: Revisit the proposal to use github PR
Posted by Jason Brown <ja...@gmail.com>.
To clarify my position: Github PRs are great for *reviewing* code, and the
commentary is much easier to follow imo. But for *merging* code, esp into
our multi-branch strategy, PRs don't fit well, unless there's some
technique I and perhaps others are unaware of.
On Thu, Dec 13, 2018 at 9:47 AM Ariel Weisberg <ar...@weisberg.ws> wrote:
> Hi,
>
> I'm not clear on what github makes worse. It preserves more history then
> the JIRA approach. When people invitably force push their branches you
> can't tell from the link to a branch on JIRA. Github preserves the comments
> and force push history so you know what version of the code each comment
> applied to. Github also tracks when requests for changes are acknowledged
> and resolved. I have had to make the same change request many times and
> keep track independently whether it was resolved. This has also resulting
> in mistakes getting merged when I missed a comment that was ignored.
>
> Now that github can CC JIRA that also CCs to commits@. It's better then
> JIRA comments because each comment includes a small diff of the code the
> comment applies to. To do that in JIRA I have to manually link to the code
> the PR and most people don't do that for every comment so some of them are
> inscrutable after the fact. Also manually created links sometimes refer to
> references that disappear or get force pushed. It's a bit tricky to get
> right.
>
> To me arguing against leveraging a better code review workflow (whether
> github or some other tool) is like arguing against using source control
> tools. Sure the filesystem and home grown scripts can be used to work
> around lack of source control, but why would you?
>
> I see two complaints so far. One is that github PRs encourage nitpicking.
> I don't see tool based solution to that off the cuff. Another is that
> github doesn't by default CC JIRA. Maybe we can just refuse to accept
> improperly formatted PRs and look into auto rejecting ones that don't refer
> to a ticket?
>
> Ariel
>
> On Thu, Dec 13, 2018, at 12:20 PM, Aleksey Yeschenko wrote:
> > There are some nice benefits to GH PRs, one of them is that we could
> > eventually set up CircleCI hooks that would explicitly prevent commits
> > that don’t pass the tests.
> >
> > But handling multiple branches would indeed be annoying. Would have to
> > either submit 1 PR per branch - which is both tedious and non-atomic -
> > or do a mixed approach, with a PR for the oldest branch, then a manual
> > merge upwards. The latter would be kinda meh, especially when commits
> > for different branches diverge.
> >
> > For me personally, the current setup works quite well, and I mostly
> > share Sylvain’s opinion above, for the same reasons listed.
> >
> > —
> > AY
> >
> > > On 13 Dec 2018, at 08:15, Sylvain Lebresne <le...@gmail.com> wrote:
> > >
> > > Fwiw, I personally find it very useful to have all discussion, review
> > > comments included, in the same place (namely JIRA, since for better or
> > > worse, that's what we use for tracking tickets). Typically, that means
> > > everything gets consistently pushed to the commits@ mailing list,
> which I
> > > find extremely convenient to keep track of things. I also have a theory
> > > that the inline-comments type of review github PR give you is very
> > > convenient for nitpicks, shallow or spur-of-the-moment comments, but
> > > doesn't help that much for deeper reviews, and that it thus to favor
> the
> > > former kind of review.
> > >
> > > Additionally, and to Benedict's point, I happen to have first hand
> > > experience with a PR-based process for a multi-branch workflow very
> similar
> > > to the one of this project, and suffice to say that I hate it with a
> > > passion.
> > >
> > > Anyway, very much personal opinion here.
> > > --
> > > Sylvain
> > >
> > >
> > > On Thu, Dec 13, 2018 at 2:13 AM dinesh.joshi@yahoo.com.INVALID
> > > <di...@yahoo.com.invalid> wrote:
> > >
> > >> I've been already using github PRs for some time now. Once you
> specify the
> > >> ticket number, the comments and discussion are persisted in Apache
> Jira as
> > >> work log so it can be audited if desired. However, committers usually
> > >> squash and commit the changes once the PR is approved. We don't use
> the
> > >> merge feature in github. I don't believe github we can merge the
> commit
> > >> into multiple branches through the UI. We would need to merge it into
> one
> > >> branch and then manually merge that commit into other branches. The
> big
> > >> upside of using github PRs is that it makes collaborating a lot
> easier.
> > >> Downside is that it makes it very difficult to follow along the
> progress in
> > >> Apache Jira. The messages that github posts back include huge diffs
> and are
> > >> aweful.
> > >> Dinesh
> > >>
> > >> On Thursday, December 13, 2018, 1:10:12 AM GMT+5:30, Benedict
> Elliott
> > >> Smith <be...@apache.org> wrote:
> > >>
> > >> Perhaps somebody could summarise the tradeoffs? I’m a little
> concerned
> > >> about how it would work for our multi-branch workflow. Would we open
> > >> multiple PRs?
> > >>
> > >> Could we easily link with external CircleCI?
> > >>
> > >> It occurs to me, in JIRA proposal mode, that an extra required field
> for a
> > >> permalink to GitHub for the patch would save a lot of time I spend
> hunting
> > >> for a branch in the comments.
> > >>
> > >>
> > >>
> > >>
> > >>> On 12 Dec 2018, at 19:20, jay.zhuang@yahoo.com.INVALID wrote:
> > >>>
> > >>> It was discussed 1 year's ago:
> > >> https://www.mail-archive.com/dev@cassandra.apache.org/msg11810.html
> > >>> As all Apache projects are moving to gitbox:
> > >> https://reference.apache.org/committer/github, should we revisit
> that and
> > >> change our review/commit process to use github PR?A good example is
> > >> Spark:"Changes to Spark source code are proposed, reviewed and
> committed
> > >> via Github pull requests" (https://spark.apache.org/contributing.html
> ).
> > >>> /jay
> > >>
> > >>
> > >> ---------------------------------------------------------------------
> > >> To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
> > >> For additional commands, e-mail: dev-help@cassandra.apache.org
> > >>
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
> > For additional commands, e-mail: dev-help@cassandra.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
> For additional commands, e-mail: dev-help@cassandra.apache.org
>
>
Re: Revisit the proposal to use github PR
Posted by Ariel Weisberg <ar...@weisberg.ws>.
Hi,
I'm not clear on what github makes worse. It preserves more history then the JIRA approach. When people invitably force push their branches you can't tell from the link to a branch on JIRA. Github preserves the comments and force push history so you know what version of the code each comment applied to. Github also tracks when requests for changes are acknowledged and resolved. I have had to make the same change request many times and keep track independently whether it was resolved. This has also resulting in mistakes getting merged when I missed a comment that was ignored.
Now that github can CC JIRA that also CCs to commits@. It's better then JIRA comments because each comment includes a small diff of the code the comment applies to. To do that in JIRA I have to manually link to the code the PR and most people don't do that for every comment so some of them are inscrutable after the fact. Also manually created links sometimes refer to references that disappear or get force pushed. It's a bit tricky to get right.
To me arguing against leveraging a better code review workflow (whether github or some other tool) is like arguing against using source control tools. Sure the filesystem and home grown scripts can be used to work around lack of source control, but why would you?
I see two complaints so far. One is that github PRs encourage nitpicking. I don't see tool based solution to that off the cuff. Another is that github doesn't by default CC JIRA. Maybe we can just refuse to accept improperly formatted PRs and look into auto rejecting ones that don't refer to a ticket?
Ariel
On Thu, Dec 13, 2018, at 12:20 PM, Aleksey Yeschenko wrote:
> There are some nice benefits to GH PRs, one of them is that we could
> eventually set up CircleCI hooks that would explicitly prevent commits
> that don’t pass the tests.
>
> But handling multiple branches would indeed be annoying. Would have to
> either submit 1 PR per branch - which is both tedious and non-atomic -
> or do a mixed approach, with a PR for the oldest branch, then a manual
> merge upwards. The latter would be kinda meh, especially when commits
> for different branches diverge.
>
> For me personally, the current setup works quite well, and I mostly
> share Sylvain’s opinion above, for the same reasons listed.
>
> —
> AY
>
> > On 13 Dec 2018, at 08:15, Sylvain Lebresne <le...@gmail.com> wrote:
> >
> > Fwiw, I personally find it very useful to have all discussion, review
> > comments included, in the same place (namely JIRA, since for better or
> > worse, that's what we use for tracking tickets). Typically, that means
> > everything gets consistently pushed to the commits@ mailing list, which I
> > find extremely convenient to keep track of things. I also have a theory
> > that the inline-comments type of review github PR give you is very
> > convenient for nitpicks, shallow or spur-of-the-moment comments, but
> > doesn't help that much for deeper reviews, and that it thus to favor the
> > former kind of review.
> >
> > Additionally, and to Benedict's point, I happen to have first hand
> > experience with a PR-based process for a multi-branch workflow very similar
> > to the one of this project, and suffice to say that I hate it with a
> > passion.
> >
> > Anyway, very much personal opinion here.
> > --
> > Sylvain
> >
> >
> > On Thu, Dec 13, 2018 at 2:13 AM dinesh.joshi@yahoo.com.INVALID
> > <di...@yahoo.com.invalid> wrote:
> >
> >> I've been already using github PRs for some time now. Once you specify the
> >> ticket number, the comments and discussion are persisted in Apache Jira as
> >> work log so it can be audited if desired. However, committers usually
> >> squash and commit the changes once the PR is approved. We don't use the
> >> merge feature in github. I don't believe github we can merge the commit
> >> into multiple branches through the UI. We would need to merge it into one
> >> branch and then manually merge that commit into other branches. The big
> >> upside of using github PRs is that it makes collaborating a lot easier.
> >> Downside is that it makes it very difficult to follow along the progress in
> >> Apache Jira. The messages that github posts back include huge diffs and are
> >> aweful.
> >> Dinesh
> >>
> >> On Thursday, December 13, 2018, 1:10:12 AM GMT+5:30, Benedict Elliott
> >> Smith <be...@apache.org> wrote:
> >>
> >> Perhaps somebody could summarise the tradeoffs? I’m a little concerned
> >> about how it would work for our multi-branch workflow. Would we open
> >> multiple PRs?
> >>
> >> Could we easily link with external CircleCI?
> >>
> >> It occurs to me, in JIRA proposal mode, that an extra required field for a
> >> permalink to GitHub for the patch would save a lot of time I spend hunting
> >> for a branch in the comments.
> >>
> >>
> >>
> >>
> >>> On 12 Dec 2018, at 19:20, jay.zhuang@yahoo.com.INVALID wrote:
> >>>
> >>> It was discussed 1 year's ago:
> >> https://www.mail-archive.com/dev@cassandra.apache.org/msg11810.html
> >>> As all Apache projects are moving to gitbox:
> >> https://reference.apache.org/committer/github, should we revisit that and
> >> change our review/commit process to use github PR?A good example is
> >> Spark:"Changes to Spark source code are proposed, reviewed and committed
> >> via Github pull requests" (https://spark.apache.org/contributing.html).
> >>> /jay
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
> >> For additional commands, e-mail: dev-help@cassandra.apache.org
> >>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
> For additional commands, e-mail: dev-help@cassandra.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
For additional commands, e-mail: dev-help@cassandra.apache.org
Re: Revisit the proposal to use github PR
Posted by Aleksey Yeschenko <al...@yeschenko.com>.
There are some nice benefits to GH PRs, one of them is that we could eventually set up CircleCI hooks that would explicitly prevent commits that don’t pass the tests.
But handling multiple branches would indeed be annoying. Would have to either submit 1 PR per branch - which is both tedious and non-atomic - or do a mixed approach, with a PR for the oldest branch, then a manual merge upwards. The latter would be kinda meh, especially when commits for different branches diverge.
For me personally, the current setup works quite well, and I mostly share Sylvain’s opinion above, for the same reasons listed.
—
AY
> On 13 Dec 2018, at 08:15, Sylvain Lebresne <le...@gmail.com> wrote:
>
> Fwiw, I personally find it very useful to have all discussion, review
> comments included, in the same place (namely JIRA, since for better or
> worse, that's what we use for tracking tickets). Typically, that means
> everything gets consistently pushed to the commits@ mailing list, which I
> find extremely convenient to keep track of things. I also have a theory
> that the inline-comments type of review github PR give you is very
> convenient for nitpicks, shallow or spur-of-the-moment comments, but
> doesn't help that much for deeper reviews, and that it thus to favor the
> former kind of review.
>
> Additionally, and to Benedict's point, I happen to have first hand
> experience with a PR-based process for a multi-branch workflow very similar
> to the one of this project, and suffice to say that I hate it with a
> passion.
>
> Anyway, very much personal opinion here.
> --
> Sylvain
>
>
> On Thu, Dec 13, 2018 at 2:13 AM dinesh.joshi@yahoo.com.INVALID
> <di...@yahoo.com.invalid> wrote:
>
>> I've been already using github PRs for some time now. Once you specify the
>> ticket number, the comments and discussion are persisted in Apache Jira as
>> work log so it can be audited if desired. However, committers usually
>> squash and commit the changes once the PR is approved. We don't use the
>> merge feature in github. I don't believe github we can merge the commit
>> into multiple branches through the UI. We would need to merge it into one
>> branch and then manually merge that commit into other branches. The big
>> upside of using github PRs is that it makes collaborating a lot easier.
>> Downside is that it makes it very difficult to follow along the progress in
>> Apache Jira. The messages that github posts back include huge diffs and are
>> aweful.
>> Dinesh
>>
>> On Thursday, December 13, 2018, 1:10:12 AM GMT+5:30, Benedict Elliott
>> Smith <be...@apache.org> wrote:
>>
>> Perhaps somebody could summarise the tradeoffs? I’m a little concerned
>> about how it would work for our multi-branch workflow. Would we open
>> multiple PRs?
>>
>> Could we easily link with external CircleCI?
>>
>> It occurs to me, in JIRA proposal mode, that an extra required field for a
>> permalink to GitHub for the patch would save a lot of time I spend hunting
>> for a branch in the comments.
>>
>>
>>
>>
>>> On 12 Dec 2018, at 19:20, jay.zhuang@yahoo.com.INVALID wrote:
>>>
>>> It was discussed 1 year's ago:
>> https://www.mail-archive.com/dev@cassandra.apache.org/msg11810.html
>>> As all Apache projects are moving to gitbox:
>> https://reference.apache.org/committer/github, should we revisit that and
>> change our review/commit process to use github PR?A good example is
>> Spark:"Changes to Spark source code are proposed, reviewed and committed
>> via Github pull requests" (https://spark.apache.org/contributing.html).
>>> /jay
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
>> For additional commands, e-mail: dev-help@cassandra.apache.org
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
For additional commands, e-mail: dev-help@cassandra.apache.org
Re: Revisit the proposal to use github PR
Posted by Sylvain Lebresne <le...@gmail.com>.
Fwiw, I personally find it very useful to have all discussion, review
comments included, in the same place (namely JIRA, since for better or
worse, that's what we use for tracking tickets). Typically, that means
everything gets consistently pushed to the commits@ mailing list, which I
find extremely convenient to keep track of things. I also have a theory
that the inline-comments type of review github PR give you is very
convenient for nitpicks, shallow or spur-of-the-moment comments, but
doesn't help that much for deeper reviews, and that it thus to favor the
former kind of review.
Additionally, and to Benedict's point, I happen to have first hand
experience with a PR-based process for a multi-branch workflow very similar
to the one of this project, and suffice to say that I hate it with a
passion.
Anyway, very much personal opinion here.
--
Sylvain
On Thu, Dec 13, 2018 at 2:13 AM dinesh.joshi@yahoo.com.INVALID
<di...@yahoo.com.invalid> wrote:
> I've been already using github PRs for some time now. Once you specify the
> ticket number, the comments and discussion are persisted in Apache Jira as
> work log so it can be audited if desired. However, committers usually
> squash and commit the changes once the PR is approved. We don't use the
> merge feature in github. I don't believe github we can merge the commit
> into multiple branches through the UI. We would need to merge it into one
> branch and then manually merge that commit into other branches. The big
> upside of using github PRs is that it makes collaborating a lot easier.
> Downside is that it makes it very difficult to follow along the progress in
> Apache Jira. The messages that github posts back include huge diffs and are
> aweful.
> Dinesh
>
> On Thursday, December 13, 2018, 1:10:12 AM GMT+5:30, Benedict Elliott
> Smith <be...@apache.org> wrote:
>
> Perhaps somebody could summarise the tradeoffs? I’m a little concerned
> about how it would work for our multi-branch workflow. Would we open
> multiple PRs?
>
> Could we easily link with external CircleCI?
>
> It occurs to me, in JIRA proposal mode, that an extra required field for a
> permalink to GitHub for the patch would save a lot of time I spend hunting
> for a branch in the comments.
>
>
>
>
> > On 12 Dec 2018, at 19:20, jay.zhuang@yahoo.com.INVALID wrote:
> >
> > It was discussed 1 year's ago:
> https://www.mail-archive.com/dev@cassandra.apache.org/msg11810.html
> > As all Apache projects are moving to gitbox:
> https://reference.apache.org/committer/github, should we revisit that and
> change our review/commit process to use github PR?A good example is
> Spark:"Changes to Spark source code are proposed, reviewed and committed
> via Github pull requests" (https://spark.apache.org/contributing.html).
> > /jay
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
> For additional commands, e-mail: dev-help@cassandra.apache.org
>
Re: Revisit the proposal to use github PR
Posted by "dinesh.joshi@yahoo.com.INVALID" <di...@yahoo.com.INVALID>.
I've been already using github PRs for some time now. Once you specify the ticket number, the comments and discussion are persisted in Apache Jira as work log so it can be audited if desired. However, committers usually squash and commit the changes once the PR is approved. We don't use the merge feature in github. I don't believe github we can merge the commit into multiple branches through the UI. We would need to merge it into one branch and then manually merge that commit into other branches. The big upside of using github PRs is that it makes collaborating a lot easier. Downside is that it makes it very difficult to follow along the progress in Apache Jira. The messages that github posts back include huge diffs and are aweful.
Dinesh
On Thursday, December 13, 2018, 1:10:12 AM GMT+5:30, Benedict Elliott Smith <be...@apache.org> wrote:
Perhaps somebody could summarise the tradeoffs? I’m a little concerned about how it would work for our multi-branch workflow. Would we open multiple PRs?
Could we easily link with external CircleCI?
It occurs to me, in JIRA proposal mode, that an extra required field for a permalink to GitHub for the patch would save a lot of time I spend hunting for a branch in the comments.
> On 12 Dec 2018, at 19:20, jay.zhuang@yahoo.com.INVALID wrote:
>
> It was discussed 1 year's ago: https://www.mail-archive.com/dev@cassandra.apache.org/msg11810.html
> As all Apache projects are moving to gitbox: https://reference.apache.org/committer/github, should we revisit that and change our review/commit process to use github PR?A good example is Spark:"Changes to Spark source code are proposed, reviewed and committed via Github pull requests" (https://spark.apache.org/contributing.html).
> /jay
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
For additional commands, e-mail: dev-help@cassandra.apache.org
Re: Revisit the proposal to use github PR
Posted by Benedict Elliott Smith <be...@apache.org>.
Perhaps somebody could summarise the tradeoffs? I’m a little concerned about how it would work for our multi-branch workflow. Would we open multiple PRs?
Could we easily link with external CircleCI?
It occurs to me, in JIRA proposal mode, that an extra required field for a permalink to GitHub for the patch would save a lot of time I spend hunting for a branch in the comments.
> On 12 Dec 2018, at 19:20, jay.zhuang@yahoo.com.INVALID wrote:
>
> It was discussed 1 year's ago: https://www.mail-archive.com/dev@cassandra.apache.org/msg11810.html
> As all Apache projects are moving to gitbox: https://reference.apache.org/committer/github, should we revisit that and change our review/commit process to use github PR?A good example is Spark:"Changes to Spark source code are proposed, reviewed and committed via Github pull requests" (https://spark.apache.org/contributing.html).
> /jay
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cassandra.apache.org
For additional commands, e-mail: dev-help@cassandra.apache.org