> What particularly do you think does not work well with it?
>

File reviews or bound to the commit id and when you force push, the review
is gone. See the screenshot below. This makes it very hard for the reviewer
to see if their suggestions have been followed.


>
>
>> I propose to just keep committing on the branch (a bit like Gerrit change
>> sets) and then always(!) Squash and Merge
>>
> As reviewers, we shouldn't have to care about it; and feel free to use
> Squash and Merge if it seems better, or even ask contributors to reorganize
> their commits before merging if it seems necessary. What's important is
> that what we merge is in good quality of code and also of history, so we
> keep the history sane. But this is not really a contributor's duty to
> achieve that, more a reviewer's one.
> Of course, we should avoid --force on the "upstream" eclipse repo.
>

Yes, sure. I just wanted to lay down my experiences. I agree that this
should be on a case by case basis but as a rule of thumb, I recommend

* No --force just keep committing as we did with Gerrit
* Squash and merge to keep the commit history clean


Take a look at this PR with a review after force push:
https://github.com/eclipse/birt/pull/886

[image: image.png]


After clicking on that file review comment:

[image: image.png]



> _______________________________________________
> platform-dev mailing list
> platform-dev@eclipse.org
> To unsubscribe from this list, visit
> https://www.eclipse.org/mailman/listinfo/platform-dev
>
_______________________________________________
platform-dev mailing list
platform-dev@eclipse.org
To unsubscribe from this list, visit 
https://www.eclipse.org/mailman/listinfo/platform-dev

Reply via email to