SGTM then, my new vote is +1 to (a) as it preserves the most information,
just the reviewer need to keep in mind to tell the author to squash the
noise commit before merge.
On Wed, Nov 29, 2017 at 12:09 PM Kenneth Knowles wrote:
> Let's assume that when I say (a) the author has arranged commits
+1 to Kenneth proposal, using reviewer and asignee, for the merge strategy
(a) +1 with the same arguments (preserving commits when they are
meaningful and isolated, ask committers to do extra squash if needed.
I don't really favor having one big commit per PR (in particular if
the change is big) b
I think I agree with Kenn on the "merge question":
- There should be a merge commit because this records important information,
for example, I like having the option of figuring out what PR certain commits
came from
- Individual meaningful commits of a PR should be preserved, I think having
co
Hi,
I don't see why gitbox merge button change what we are doing.
I agree with Kenn for 1 (reviewer field) & 2 (assignee field).
IMHO, for 3, I think the reviewer should only use rebase & merge. The squash
should be under the contributor scope. The reviewer can ask to squash some
commits, but
Let's assume that when I say (a) the author has arranged commits to be
meaningful. That's what I meant to say in each of my descriptions of the
option. If they are noise, it doesn't apply.
On Tue, Nov 28, 2017 at 8:04 PM, James wrote:
> Thanks Kenn for bring up this expanded discussion, my vote
Thanks Kenn for bring up this expanded discussion, my vote is:
(a) -1 this preserves noise log like 'fix review comments'
(b) +0 this keeps the commit log clean, but without a rebase
(c) -1 similar to option a), it preserves noise log like 'fix review
comments'
My ideal option is the current manu
On Tue, Nov 28, 2017 at 11:47 AM, Thomas Weise wrote:
>
> (a) -0 due to extra noise in the commit log
>
> (b) -1 (as standard/default) this should be done by contributor as there
> may be few situation where individual commits should be preserved
>
It is better to preserve the commit history o
I prefer either (a) or (b) depending on the content of the commit
history. Some PRs have well-curated, useful distinct commits, whereas
others consist mostly of fixups and other minor changes that are
better squashed.
On Tue, Nov 28, 2017 at 11:47 AM, Thomas Weise wrote:
>
> (a) -0 due to extra n
(a) -0 due to extra noise in the commit log
(b) -1 (as standard/default) this should be done by contributor as there
may be few situation where individual commits should be preserved
(c) +1 the rebase will also record the committer (which would be merge
commit author otherwise)
In general the proc
On Tue, Nov 28, 2017 at 11:16 AM, Raghu Angadi wrote:
> -1 for (a): no need to see all the private branch commits from
> contributor. It often makes me more conscious of local commits.
>
I want to note that on my PRs these are not private commits. Each one is a
meaningful isolated change that ca
-1 for (a): no need to see all the private branch commits from contributor.
It often makes me more conscious of local commits.
+1 for (b): with committer replacing the squashed commit messages with
'[BEAM-jira or PRID]: Brief cut-n-paste (or longer if it contributor
provided one)'.
-1 for (c): This
Is it possible for mergebot to auto squash any fixup! and perform the merge
commit as described in (a), if so then I would vote for mergebot.
Without mergebot, I vote:
(a) 0 I like squashing fixup!
(b) -1
(c) +1 Most of our PRs are for focused singular changes which is why I
would rather squash ev
On Tue, Nov 28, 2017 at 9:51 AM, Ben Chambers wrote:
> One risk to "squash and merge" is that it may lead to commits that don't
> have clean descriptions -- for instance, commits like "Fixing review
> comments" will show up. If we use (a) these would also show up as separate
> commits. It seems l
I am strongly in favor of (1); I have no strong feelings about (2); I agree
on (3), but generically am not hugely concerned, so long as back-references
to the original PR are maintained, which is where most of the context
lives. It is nice to have the change broken up into as many individually
usef
One risk to "squash and merge" is that it may lead to commits that don't
have clean descriptions -- for instance, commits like "Fixing review
comments" will show up. If we use (a) these would also show up as separate
commits. It seems like there are two cases of multiple commits in a PR:
1. Multip
Hi,
In other Apache projects using gitbox, I experiment, the following cinematic:
1. use the review button to assign someone
2. once changes approved, I use the merge button (supporting squash and merge)
It's very convenient and works fine.
So, +1 to (b)
Regards
JB
On 11/28/2017 06:45 PM, Ke
Hi all,
James brought up a great question in Slack, which was how should we use the
merge button, illustrated [1]
I want to broaden the discussion to talk about all the new capabilities:
1. Whether & how to use the "reviewer" field
2. Whether & how to use the "assignee" field
3. Whether & how to
17 matches
Mail list logo
