Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [rdf4j-dev] Git history and merging - again

Smugness will happily be tolerated :)

Thanks for turning it back on. I’ll do a test to see how my email works when merging. 

Håvard

On 20 Jan 2021, at 11:01, Jeen Broekstra <jeen@xxxxxxxxxxxx> wrote:


I have confirmation from the Github tech team that indeed, Github uses the primary e-mail from the account that created the PR as the author on a squash-and-merge commit (and noreply.github.com if the account is configured to keep e-mail private).

Since there is no way to know what this is set to in advance (other than asking the author about it), it means that squash-and-merge has only very limited usability for us:

1. we can not use it as a merge technique for PRs created by third party contributors unless we have explicit confirmation from those contributors how their account is configured (and to be honest, it feels a bit awkward to ask);
2. we can not use it as a merge technique for any PRs created by myself or by Andreas, because our primary e-mail addresses are not the addresses we want associated with RDF4J commits.

I will re-enable it because I'm sick of fighting about this, but I want it understood that I am not really in favor of its use. It's a poor way of keeping track of IP, and I dislike the fact that we have two methods for merging PRs depending on who created the PR - it's bound to cause confusion.

I suggest any of you use it very sparingly, in cases where you're sure the PR author is correctly configured, where a PR really needs to be squashed but manual squashing is turning out to be hard because of conflicts. I still expect contributors to make a genuine effort to keep their branch history clean, well described, and self-contained so that we can merge the majority of things with merge commits. 

Oh and Havard: I also reserve the right to smugly tell you "I told you so" when the master build breaks a few months from now on a failing ECA check ;)

Jeen

On Wed, 20 Jan 2021, at 01:25, Jeen Broekstra wrote:
On Tue, 19 Jan 2021, at 23:31, Håvard Ottestad wrote:

I did try to rebase it. But just on the first commit I got 7 merge conflicts for files I hadn’t even changed.

Fwiw on another look at that PR, I don't think it is really necessary to squash it, after all: there's only few commits and they're all meaningfullly commented - I think I mistook the merge commits in there for commits missing an issue number.

That said, (and I realize it's after the fact) I still don't see why you find it so hard to use rebase instead of merge to keep your feature branch in sync. It isn't hard. I've switched to it. Other contributors seem to have little trouble with it. This is what annoyed me about your mail initially - I got the feeling that you weren't really trying. Perhaps that's unkind of me but I hope you understand my frustration too: I put a lot of energy in discussing and documenting this, and making it as easy as possible. It's not nice for me to then hear you say it's "too strict" and "completely unnecessary". 

If it really is that dififcult and frustrating though, let's see what we can do to make it less of a hurdle.

My experience tells me that using effort on these things is not worth it.

If we want the history to be as compressed and simple as possible we should use the squash and merge button for those 99% of the cases. We will lose a lot of value though. Since that bug fix commit in the middle of that medium size PR gets lost next time someone wants to figure out why some strange code is kinda funky.

That is actually an argument for sticking with merge-commits, and perhaps being a bit less trigger-happy on the manual squashing. 
 
But if eclipse says that they don’t minI'md the email getting a bit mucked up for merges, then why should we. The signoff will still be there.
 

Just to be clear: Eclipse find an occassional noreply.github.com address slipping in acceptable. They (and me as well) are most definitely not ok with the author address changing to a different address than what they have on the ECA record (e.g. bob@work.example on the PR merge when bob only signed the ECA for bob@personal.example) just because bob has their account configured that way. And like I said: you can't see this in advance. That's the problem.

I think having more choice is better. 

The reason I keep trying to pick one strategy is that I'm trying to keep things simple and consistent. A git history that uses a single strategy is easier to read. Having both options enabled will inevitably lead to someone picking the wrong one at some point (perhaps not the end of the world, but still..).

But if you're saying we should use more individual discretion, then fine: we can of course enable both merge-commit and squash-and-merge, and use each in diffferent situations: squash and merge on big PRs with lots of commits that we want to squash where we know the author is correctly configured, merge commits on everything else (that will mean most third party contributions), occassional manual squashing/rebasing if the situation demands it (perhaps only if there are mulitple commits with unclear messaging in there). 

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


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

Back to the top