Proposal: Immediate Deprecation of ChangeLogs
A few weeks ago, I started a discussion about deprecating ChangeLogs. In that time, we’ve had more folks using the pull-request workflow and more folks using newer versions of `git` which break automatic ChangeLog rebasing. I propose that on Monday, May 16th, we implement the following policy changes for the WebKit project: - Commits no longer require ChangeLogs, they instead require commit messages - Commit messages are in the format of `prepare-ChangeLog --no-write` Pull-request workflows based on `git-webkit` already support this workflow well, and `git-webkit setup` creates a `prepare-commit-msg` hook that will appropriately format commit messages. In addition, `git format-patch` allows us to create a patch which contains a commit message. This means that contributors still using patch workflows from a git or git-svn checkout will be able to upload compliant patches to bugzilla. This will, however, break contributors using pure-Subversion checkouts. This is something that’s going to be happening in the very near future as we deprecate Subversion entirely, so I think this is an acceptable cost in exchange for fully supporting native git workflows. The last thing I’d like to note is that a full git-native commit message policy now is something we can modify in the future if we find that reviewing commit messages with “Quote reply” comments is not sufficient, but resolving project disagreements on how or if to address deficiencies in GitHub commit message review don’t seem to be headed towards a resolution quickly. Jonathan WebKit Continuous Integration
I don't think we should this. We haven't even had enough discussions about whether we want to deprecate change logs or not. On Tue, May 10, 2022 at 13:32 Jonathan Bedard via webkit-dev < webkit-dev@lists.webkit.org> wrote:
A few weeks ago, I started a discussion about deprecating ChangeLogs. In that time, we’ve had more folks using the pull-request workflow and more folks using newer versions of `git` which break automatic ChangeLog rebasing. I propose that on Monday, May 16th, we implement the following policy changes for the WebKit project:
- Commits no longer require ChangeLogs, they instead require commit messages - Commit messages are in the format of `prepare-ChangeLog --no-write`
Pull-request workflows based on `git-webkit` already support this workflow well, and `git-webkit setup` creates a `prepare-commit-msg` hook that will appropriately format commit messages. In addition, `git format-patch` allows us to create a patch which contains a commit message. This means that contributors still using patch workflows from a git or git-svn checkout will be able to upload compliant patches to bugzilla.
This will, however, break contributors using pure-Subversion checkouts. This is something that’s going to be happening in the very near future as we deprecate Subversion entirely, so I think this is an acceptable cost in exchange for fully supporting native git workflows.
The last thing I’d like to note is that a full git-native commit message policy now is something we can modify in the future if we find that reviewing commit messages with “Quote reply” comments is not sufficient, but resolving project disagreements on how or if to address deficiencies in GitHub commit message review don’t seem to be headed towards a resolution quickly.
Jonathan WebKit Continuous Integration
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
-- - R. Niwa
Hi, I think this is a step in the right direction. I hope concerns from other contributors about change log reviews can be addressed in the near future. However, I don’t think it should prevent us from moving away from ChangeLog files, given that commenting on commit logs is still possible in GitHub, although not conveniently. Thanks,
On May 10, 2022, at 1:32 PM, Jonathan Bedard via webkit-dev <webkit-dev@lists.webkit.org> wrote:
A few weeks ago, I started a discussion about deprecating ChangeLogs. In that time, we’ve had more folks using the pull-request workflow and more folks using newer versions of `git` which break automatic ChangeLog rebasing. I propose that on Monday, May 16th, we implement the following policy changes for the WebKit project:
- Commits no longer require ChangeLogs, they instead require commit messages - Commit messages are in the format of `prepare-ChangeLog --no-write`
Pull-request workflows based on `git-webkit` already support this workflow well, and `git-webkit setup` creates a `prepare-commit-msg` hook that will appropriately format commit messages. In addition, `git format-patch` allows us to create a patch which contains a commit message. This means that contributors still using patch workflows from a git or git-svn checkout will be able to upload compliant patches to bugzilla.
This will, however, break contributors using pure-Subversion checkouts. This is something that’s going to be happening in the very near future as we deprecate Subversion entirely, so I think this is an acceptable cost in exchange for fully supporting native git workflows.
The last thing I’d like to note is that a full git-native commit message policy now is something we can modify in the future if we find that reviewing commit messages with “Quote reply” comments is not sufficient, but resolving project disagreements on how or if to address deficiencies in GitHub commit message review don’t seem to be headed towards a resolution quickly.
Jonathan WebKit Continuous Integration
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Do I undertand correctly that the proposal here is (a) Immediately Deprecate ChangeLogs (b) Immediately end support for posting patches from Subversion checkouts? If so, do you know how many regular WebKit contributors still post patches from Subversion checkouts, and, if that number is not zero, what their schedule is for migrating to git, and whether they need anything from our tools engineers to make that migration smooth? Seems… problematically forward-looking… to propose immediate migration without that data. Thanks, Geoff
On May 10, 2022, at 1:32 PM, Jonathan Bedard via webkit-dev <webkit-dev@lists.webkit.org> wrote:
A few weeks ago, I started a discussion about deprecating ChangeLogs. In that time, we’ve had more folks using the pull-request workflow and more folks using newer versions of `git` which break automatic ChangeLog rebasing. I propose that on Monday, May 16th, we implement the following policy changes for the WebKit project:
- Commits no longer require ChangeLogs, they instead require commit messages - Commit messages are in the format of `prepare-ChangeLog --no-write`
Pull-request workflows based on `git-webkit` already support this workflow well, and `git-webkit setup` creates a `prepare-commit-msg` hook that will appropriately format commit messages. In addition, `git format-patch` allows us to create a patch which contains a commit message. This means that contributors still using patch workflows from a git or git-svn checkout will be able to upload compliant patches to bugzilla.
This will, however, break contributors using pure-Subversion checkouts. This is something that’s going to be happening in the very near future as we deprecate Subversion entirely, so I think this is an acceptable cost in exchange for fully supporting native git workflows.
The last thing I’d like to note is that a full git-native commit message policy now is something we can modify in the future if we find that reviewing commit messages with “Quote reply” comments is not sufficient, but resolving project disagreements on how or if to address deficiencies in GitHub commit message review don’t seem to be headed towards a resolution quickly.
Jonathan WebKit Continuous Integration
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
On May 10, 2022, at 2:46 PM, Geoffrey Garen <ggaren@apple.com> wrote:
Do I undertand correctly that the proposal here is
(a) Immediately Deprecate ChangeLogs
Yes
(b) Immediately end support for posting patches from Subversion checkouts?
We would be immediately ending support for _landing_ patches posted from a Subversion checkout. EWS would continue to accept and test patches posted from Subversion checkouts.
If so, do you know how many regular WebKit contributors still post patches from Subversion checkouts, and, if that number is not zero, what their schedule is for migrating to git, and whether they need anything from our tools engineers to make that migration smooth?
We don’t know how many contributors still post patches from Subversion checkouts, and we actually can’t easily answer this question because we’ve had feature parity between Subversion and git-svn checkouts for many years now. There are a few types of changes that aren’t compatible with the patch workflow in general, but those types of changes are a) rare and b) supported in the pull-request workflow
Seems… problematically forward-looking… to propose immediate migration without that data.
Thanks, Geoff
On May 10, 2022, at 1:32 PM, Jonathan Bedard via webkit-dev <webkit-dev@lists.webkit.org> wrote:
A few weeks ago, I started a discussion about deprecating ChangeLogs. In that time, we’ve had more folks using the pull-request workflow and more folks using newer versions of `git` which break automatic ChangeLog rebasing. I propose that on Monday, May 16th, we implement the following policy changes for the WebKit project:
- Commits no longer require ChangeLogs, they instead require commit messages - Commit messages are in the format of `prepare-ChangeLog --no-write`
Pull-request workflows based on `git-webkit` already support this workflow well, and `git-webkit setup` creates a `prepare-commit-msg` hook that will appropriately format commit messages. In addition, `git format-patch` allows us to create a patch which contains a commit message. This means that contributors still using patch workflows from a git or git-svn checkout will be able to upload compliant patches to bugzilla.
This will, however, break contributors using pure-Subversion checkouts. This is something that’s going to be happening in the very near future as we deprecate Subversion entirely, so I think this is an acceptable cost in exchange for fully supporting native git workflows.
The last thing I’d like to note is that a full git-native commit message policy now is something we can modify in the future if we find that reviewing commit messages with “Quote reply” comments is not sufficient, but resolving project disagreements on how or if to address deficiencies in GitHub commit message review don’t seem to be headed towards a resolution quickly.
Jonathan WebKit Continuous Integration
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
On Tue, May 10, 2022 at 3:01 PM Jonathan Bedard via webkit-dev <webkit-dev@lists.webkit.org> wrote:
On May 10, 2022, at 2:46 PM, Geoffrey Garen <ggaren@apple.com> wrote:
Do I undertand correctly that the proposal here is
(a) Immediately Deprecate ChangeLogs
Yes
(b) Immediately end support for posting patches from Subversion checkouts?
We would be immediately ending support for _landing_ patches posted from a Subversion checkout. EWS would continue to accept and test patches posted from Subversion checkouts.
Just this week, I landed 2~3 patches using a pure Subversion checkout. It's actually my primary method of landing patches in WebKit right now. - R. Niwa
[Not sure why Apple Mail sent Ryosuke’s replies to the Junk folder but I finally noticed.]
On May 10, 2022, at 3:04 PM, Ryosuke Niwa via webkit-dev <webkit-dev@lists.webkit.org> wrote:
On Tue, May 10, 2022 at 3:01 PM Jonathan Bedard via webkit-dev <webkit-dev@lists.webkit.org> wrote:
On May 10, 2022, at 2:46 PM, Geoffrey Garen <ggaren@apple.com> wrote:
Do I undertand correctly that the proposal here is
(a) Immediately Deprecate ChangeLogs
Yes
(b) Immediately end support for posting patches from Subversion checkouts?
We would be immediately ending support for _landing_ patches posted from a Subversion checkout. EWS would continue to accept and test patches posted from Subversion checkouts.
Just this week, I landed 2~3 patches using a pure Subversion checkout. It's actually my primary method of landing patches in WebKit right now.
Do you feel like 1 week is not enough time for you to do a git checkout and familiarize yourself enough with GIT to upload patches? Is that the issue? If so, how long do you feel would be reasonable? If you’re not ready to adopt the GitHub workflow for a reason or another, git-svn / bugzilla patches is still a thing and will still work for now. Only committing from pure SVN repositories would go away in a week.
- R. Niwa _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
On May 10, 2022, at 1:32 PM, Jonathan Bedard via webkit-dev <webkit-dev@lists.webkit.org> wrote:
The last thing I’d like to note is that a full git-native commit message policy now is something we can modify in the future if we find that reviewing commit messages with “Quote reply” comments is not sufficient,
I'm happy that this is noted explicitly! Do we have some of candidate plans that make reviewing experience better? For reviewing, to me, this is the only problem in GitHub. -Yusuke
Jonathan WebKit Continuous Integration
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Another idea (not formally proposing): Safari extension to bring Bugzilla's good points to GitHub UI. I tried writing random userscripts, and while threaded comments for commit message / conversation view while reviewing are still missing, I got some UI to make comment on commit message easier while reviewing. Good thing is that we can bring interesting things to GitHub without changing underlying protocol much. Bad thing is it can be fragile. Anyway, I hope we can have a solution :) -Yusuke
On May 10, 2022, at 8:50 PM, Yusuke Suzuki via webkit-dev <webkit-dev@lists.webkit.org> wrote:
On May 10, 2022, at 1:32 PM, Jonathan Bedard via webkit-dev <webkit-dev@lists.webkit.org> wrote:
The last thing I’d like to note is that a full git-native commit message policy now is something we can modify in the future if we find that reviewing commit messages with “Quote reply” comments is not sufficient,
I'm happy that this is noted explicitly! Do we have some of candidate plans that make reviewing experience better? For reviewing, to me, this is the only problem in GitHub.
-Yusuke
Jonathan WebKit Continuous Integration
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Trying to embed previous replies is going to get messy, will be referencing those replies but not embedding them. Unsafe-Merge-Queue should be very fast, I haven’t seen anything take longer than 10 minutes from label application to landing or rejection. The average case is 3-4 minutes. We’re aware of what the architectural problem with Commit-Queue is that slows down the fast path, Unsafe-Merge-Queue has fixed that. The solution we used isn’t transferable to Commit-Queue. Manual landing from a Subversion checkout isn’t broken by this proposal (although that is coming very soon), but it will be made much more painful. In my response to Geoff, I said: We would be immediately ending support for _landing_ patches posted from a Subversion checkout This is a deliberately narrow statement. Because SVN doesn’t have local commit messages, we can’t generate (or apply) patches containing commit messages. You can still land from a raw Subversion checkout, but you would need to manually draft a compliant commit message. `git svn dcommit` from a git-svn checkout is also unaffected (and what Commit-Queue and Merge-Queue are using and will continue to use) because git checkouts can apply patches with commit messages. To R. Niwa’s point that he would “never want to make a local commit”, that’s not going to be possible in the very near future, and there isn’t much we can do tooling wise to change this fact. As mentioned in my initial email, discussions around placing commit messages in files prior to merge for review don’t seem to be headed towards a resolution quickly, and the active harm ChangeLogs are causing to development is making their deprecation more urgent than the aforementioned discussion would seem to be resolving. Lastly, Chris did mention this, but just to confirm what he said, GitHub checkouts can use `git svn` too, there are a few more “yes, buts” to it (`git svn rebase` is to be avoided), as Chris mentioned, `git-webkit setup-git-svn` does work in GitHub checkouts.
On May 11, 2022, at 11:56 AM, Jonathan Bedard via webkit-dev <webkit-dev@lists.webkit.org> wrote:
Trying to embed previous replies is going to get messy, will be referencing those replies but not embedding them.
Unsafe-Merge-Queue should be very fast, I haven’t seen anything take longer than 10 minutes from label application to landing or rejection. The average case is 3-4 minutes. We’re aware of what the architectural problem with Commit-Queue is that slows down the fast path, Unsafe-Merge-Queue has fixed that. The solution we used isn’t transferable to Commit-Queue.
I personally don’t think that delaying a critical build fix / gardening by 10 minutes is OK. It’s called “unsafe-merge-queue”, why does it take this long to commit? Why isn't it as fast as "generate a unique identifier and git push”? Sure, I have no idea how the unsafe-merge-queue does but it is hard for me to imagine it should take more than 1 minute given what it needs to do and given that it should need to do extremely little testing given that it is “unsafe”. 4 to 10 minutes is nowhere near as fast as `git svn dcommit` and is therefore a pretty large regression.
I’m measuring from “engineer tells GitHub to land a change”, not “Merge-Queue checks out the change”. The basic steps of the process are: - GitHub sends hook to buildbot - buildbot validates hook and PR - buildbot checks out PR - buildbot inserts reviewer to commit message and ChangeLog - buildbot runs basic pre-commit checks - buildbot adds identifier - buildbot commits - buildbot updates PR with landed commit - buidbot comments on PR and bug with information on landed commit - buildbot closes associated bug The “commit” step usually happens at 2 minute mark currently, I gave the time for the whole process (including bug comments, bug closing, etc). It will be a bit faster once we can drop the `git-svn` pieces of this, it’s the “checkout” and “add identifier” step that can take longer than you would expect. 10 minutes is a worst-case scenario when there are already other changes in the unsafe queue and some bots are busy with Merge-Queue and Commit-Queue requests, so there is only a single available bot serving the unsafe queue. Jonathan
On May 11, 2022, at 4:31 PM, Chris Dumez <cdumez@apple.com> wrote:
On May 11, 2022, at 11:56 AM, Jonathan Bedard via webkit-dev <webkit-dev@lists.webkit.org> wrote:
Trying to embed previous replies is going to get messy, will be referencing those replies but not embedding them.
Unsafe-Merge-Queue should be very fast, I haven’t seen anything take longer than 10 minutes from label application to landing or rejection. The average case is 3-4 minutes. We’re aware of what the architectural problem with Commit-Queue is that slows down the fast path, Unsafe-Merge-Queue has fixed that. The solution we used isn’t transferable to Commit-Queue.
I personally don’t think that delaying a critical build fix / gardening by 10 minutes is OK. It’s called “unsafe-merge-queue”, why does it take this long to commit? Why isn't it as fast as "generate a unique identifier and git push”? Sure, I have no idea how the unsafe-merge-queue does but it is hard for me to imagine it should take more than 1 minute given what it needs to do and given that it should need to do extremely little testing given that it is “unsafe”.
4 to 10 minutes is nowhere near as fast as `git svn dcommit` and is therefore a pretty large regression.
participants (5)
-
Chris Dumez
-
Geoffrey Garen
-
Jonathan Bedard
-
Ryosuke Niwa
-
Yusuke Suzuki