Re: [webkit-dev] Embedding Identifiers in Commit Messages
We appreciate everyone’s feedback on transitioning away from Subversion to Git, I’ll be releasing an expected timeline of up-coming changes in the next week before the contributors meeting.
In the mean time, we’re preparing on adding identifiers to new commit messages, that work is tracked in https://bugs.webkit.org/show_bug.cgi?id=218407. <https://bugs.webkit.org/show_bug.cgi?id=218407.> At the moment, we’re likely going to be appending the identifiers to commit messages (as the current change proposes), but I wanted to provide a chance for folks to object to this change before it becomes canonical.
I'm a bit confused here. It looks like the patch only affects commits made via webkit-patch. Given there are a lot of people who don't use webkit-patch land, I'm not certain this strategy is sound even in the short term. Furthermore, the proposed patch seems to have a race condition when multiple commits are made concurrency? Why don't we do this in post commit hook instead?
- R. Niwa We don’t have post commit hooks in SVN to do this sort of thing, and I don’t intend to add them now. We are going to have a system on GitHub to do this (not post commit hooks, but I won’t dive into the details here).
There really aren’t a lot of people who land changes outside of webkit-patch, among things that would break if folks were regularly not using webkit-patch is trac.webkit.org <http://trac.webkit.org/>, which relies on the commit message being set. The types of changes that would fall outside of this change would be ones that don’t map well to a git world anyways, like the creation of a tag or branch. Notably, commit-queue is using this script. Lastly, this doesn’t add a race-condition that wasn’t already there. One of the downsides of SVN is that, unlike git, it is a centralized version control system, so clients must be synced to upstream before committing. This is true now, even if you haven’t noticed it. If we didn’t have this race condition, our changeling history would be full of weird conflicts. Jonathan
On Wed, Nov 4, 2020 at 11:51 am, Jonathan Bedard <jbedard@apple.com> wrote:
We don’t have post commit hooks in SVN to do this sort of thing, and I don’t intend to add them now. We are going to have a system on GitHub to do this (not post commit hooks, but I won’t dive into the details here).
There really aren’t a lot of people who land changes outside of webkit-patch, among things that would break if folks were regularly not using webkit-patch is trac.webkit.org, which relies on the commit message being set.
Probably not often on trunk. But on stable branches, I assume 100% of changes are landed without webkit-patch? At least, I always used 'git svn dcommit' on stable branches. I also used this on trunk when I needed to fix an error in a ChangeLog (something webkit-patch is not good at doing).
Lastly, this doesn’t add a race-condition that wasn’t already there. One of the downsides of SVN is that, unlike git, it is a centralized version control system, so clients must be synced to upstream before committing. This is true now, even if you haven’t noticed it. If we didn’t have this race condition, our changeling history would be full of weird conflicts.
There should be no race condition because our GitHub repo should only allow fast-forward commits. A server hook can ensure the commit identifiers are sequential. Right?
Forgot to update this thread: taking a different approach, will be tracked by the same bug. Will answer inline.
On Nov 11, 2020, at 7:56 AM, Michael Catanzaro <mcatanzaro@gnome.org> wrote:
On Wed, Nov 4, 2020 at 11:51 am, Jonathan Bedard <jbedard@apple.com> wrote:
We don´t have post commit hooks in SVN to do this sort of thing, and I don´t intend to add them now. We are going to have a system on GitHub to do this (not post commit hooks, but I won´t dive into the details here). There really aren´t a lot of people who land changes outside of webkit-patch, among things that would break if folks were regularly not using webkit-patch is trac.webkit.org, which relies on the commit message being set.
Probably not often on trunk. But on stable branches, I assume 100% of changes are landed without webkit-patch? At least, I always used 'git svn dcommit' on stable branches. I also used this on trunk when I needed to fix an error in a ChangeLog (something webkit-patch is not good at doing).
Turns out, directly using svn commit (or git svn dcommit) was far more common than I originally thought. We can edit SVN changelogs after a change has landed, that’s what I’m intending on doing.
Lastly, this doesn´t add a race-condition that wasn´t already there. One of the downsides of SVN is that, unlike git, it is a centralized version control system, so clients must be synced to upstream before committing. This is true now, even if you haven´t noticed it. If we didn´t have this race condition, our changeling history would be full of weird conflicts.
There should be no race condition because our GitHub repo should only allow fast-forward commits. A server hook can ensure the commit identifiers are sequential. Right?
Server hooks on GitHub are a bit difficult because GitHub (understandably) doesn’t let you run arbitrary code inside a commit action. Most projects handle this by having a merge-queue that runs code prior to landing a change (which is what we’re intending on doing).
participants (2)
-
Jonathan Bedard
-
Michael Catanzaro