Re: [webkit-dev] ChangeLog Deprecation Plan
On Mon, Apr 25, 2022 at 9:49 AM Jonathan Bedard via webkit-dev wrote: > > .. or robust solution. Bots need maintenance and intervention, and a bot with > commit access has another set of issues. Repository admins occasionally > rotating ChangeLogs is going to be less expensive than a bot doing it. > > Either way, it doesn't seem like this is a higher cost issue compared to > all WebKit contributors having to change their workflows. > > We are moving to git. Workflows _are_ changing. I’m interested in making sure > the workflows can achieve the same things the old ones did, not in preserving > the old workflows. > Just because other parts of the workflow are changing, it doesn't mean we should pile on even more changes. Honestly, this whole discussion loses me hope that the WebKit project won't remain something I want to keep contributing to in the future. > folks familiar with git won?t realize what has happened until they see > their PR diff), ?mandatory? is a bit tougher because we can?t prevent folks > from filing a pull request without using tooling. At that point, we?re > where we are today with PR template that encourages tool usage, explains > how to craft a PR the ways tools do it and then reviewers acting as a > gate-keeper. The last point is more about project policy than it is tooling. > > We should automatically reject such PR requests. > > We should not automatically reject PRs made in good faith. > > One of the reasons we’re moving to GitHub is to make the project more > approachable for new contributors. Having a machine rejects PRs because a > contributor is not familiar with project norms goes against this goal. We need to. Otherwise, support for COMMIT_MESSAGE files would be pretty much meaningless. I need those files as a reviewer, not as a patch author. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] ChangeLog Deprecation Plan
> It seems to me that you can write a script that just applies a patch while > ignoring changes to each change log file, then separately prepend each > change log entry. I don't see why such a script would behave differently > for reverting multiple commits or rebasing a feature onto a branch. In > those cases where codebase has enough divergence, merge conflicts are > probably more of an issue with the codebase itself, and not change log > entries. There are many types of changes which are not compatible with patches. “Write a change which applies a patch” is not a general solution. It wasn’t a general solution in Subversion, and it’s not a general solution in git. Speaking as the person who has to maintain our patch workflows, there are many edge cases to patch workflows that tend to bite you with precisely the kind of large changes which are difficult to manually sift through. Patch workflows also represent a huge risk for Apple’s software update branches where changes are cherry-picked by engineers who aren’t familiar with the technical details of the changes their cherry-picking. These workflows have already been using git for years at this point because patches do not work reliably for this type of work. > .. or robust solution. Bots need maintenance and intervention, and a bot with > commit access has another set of issues. Repository admins occasionally > rotating ChangeLogs is going to be less expensive than a bot doing it. > > Either way, it doesn't seem like this is a higher cost issue compared to > all WebKit contributors having to change their workflows. We are moving to git. Workflows _are_ changing. I’m interested in making sure the workflows can achieve the same things the old ones did, not in preserving the old workflows. > > folks familiar with git won?t realize what has happened until they see > their PR diff), ?mandatory? is a bit tougher because we can?t prevent folks > from filing a pull request without using tooling. At that point, we?re > where we are today with PR template that encourages tool usage, explains > how to craft a PR the ways tools do it and then reviewers acting as a > gate-keeper. The last point is more about project policy than it is tooling. > > We should automatically reject such PR requests. We should not automatically reject PRs made in good faith. One of the reasons we’re moving to GitHub is to make the project more approachable for new contributors. Having a machine rejects PRs because a contributor is not familiar with project norms goes against this goal. > … judgments about personal preferences for local development. From my > discussions with folks, it seems that the project is pretty evenly split on > --amend commits, with some contributors preferring them and others > preferring to make multiple commits. My purpose in bringing up --amend > commits is that one of the things I?ve heard folks find useful about > ChangeLogs is the ability to iteratively build them, which is something > that does have a parallel in raw git workflows. > > No, my option isn't between amending a commit vs. making multiple commits. > I really don't like making *any local commits* at all. I want to work on a > patch without ever making local commits, and upload whatever change I may > have relative to the main branch's HEAD is. Our tooling supports and will continue to support workflows where the tooling automatically makes commits for you. Discussions of other workflows is important because we don’t want to build extra tooling for the project if there is already a native git workflow which address a specific concern. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] ChangeLog Deprecation Plan
On Tue, Apr 19, 2022 at 9:33 AM Jonathan Bedard via webkit-dev < webkit-dev@lists.webkit.org> wrote: > > > To: Jonathan Bedard > > Cc: WebKit Development > > Subject: Re: [webkit-dev] ChangeLog Deprecation Plans > > Message-ID: > > < cabnrm613onu_8wjxeuhjvoh7vztc3t9r_ylnqgoj5if5as0...@mail.gmail.com> > > Content-Type: text/plain; charset="utf-8" > > > > On Mon, Apr 18, 2022 at 8:30 AM Jonathan Bedard via webkit-dev < > > webkit-dev@lists.webkit.org> wrote: > > > >> As we migrate WebKit from Subversion to git, I would like to migrate the > >> project away from ChangeLogs. The reason for this is that ChangeLogs make > >> some of the features of git hard to use, namely, cherry-picking commits > >> between branches requires conflict resolution every time. > >> > > > > Isn't this something that can be easily resolved with a merge script? > > For simple cases, yes. But with something like a multi-commit revert or rebasing a feature onto a different branch, merge scripts stop working pretty quickly. It’s usually, It might be possible to write a merge script to handle these cases, but it’s by no means “easy” in the general case. It’s also worth noting that the cases that are more complicated also tend to be the ones with the most urgency attached to them. It seems to me that you can write a script that just applies a patch while ignoring changes to each change log file, then separately prepend each change log entry. I don't see why such a script would behave differently for reverting multiple commits or rebasing a feature onto a branch. In those cases where codebase has enough divergence, merge conflicts are probably more of an issue with the codebase itself, and not change log entries. > > Rotating ChangeLogs is also moderately difficult in git with locked down > >> commit access like our project has, only repository administers would, in > >> practice, be able to rotate ChangeLogs. > >> > > > > It seems like we can just automate this by introducing a change log > > rotating bot, which has the same privilege as commit queue. > > Anything that starts with “we can just have a bot which…” is not a simple or robust solution. Bots need maintenance and intervention, and a bot with commit access has another set of issues. Repository admins occasionally rotating ChangeLogs is going to be less expensive than a bot doing it. Either way, it doesn't seem like this is a higher cost issue compared to all WebKit contributors having to change their workflows. > > So these two arguments seem rather weak. > > > > Lastly, ChangeLogs are uncommon in git based projects, so new contributors > >> will find them difficult to manage. > >> > > > > This might be the strongest argument in favor of deprecating change logs. > > > > 2) We need a way to comment on commit messages in review > >> Current tooling sets the pull request description as the commit message, > >> ?Quote Reply? kind of provides a way to inline comment, although it?s not > >> the formal review UI > >> *Proposal*: Tooling should support a ?COMMIT_MESSAGE? file in each pull > >> request commit that becomes COMMIT_MESSAGE when a pull request is landed > >> > > > > This needs to be a mandatory / automatic system, not opt-in. I want to > > comment on commit messages as a reviewer. As a patch author, I don't care > > whether it can be easily commented on or not. > > > > But if this is a required thing, then new contributors would have to learn > > that this file is auto-generated or needs to be edited manually in some > > cases so getting rid of change logs may not necessarily reduce the > > cognitive load in comparison to keeping the status quo (i.e. keeping change > > log files). > > We can make it automatic for folks using tooling (and in such a way that folks familiar with git won’t realize what has happened until they see their PR diff), “mandatory” is a bit tougher because we can’t prevent folks from filing a pull request without using tooling. At that point, we’re where we are today with PR template that encourages tool usage, explains how to craft a PR the ways tools do it and then reviewers acting as a gate-keeper. The last point is more about project policy than it is tooling. We should automatically reject such PR requests. > > 3) Edit commit messages while creating a change, not just when committing > >> the change > >> The ?overwrite? workflow already sort of support this idea by using amend > >> commits instead of appending commits to an existing branch > >> *Proposal*: The above ?COMMIT_MESSAGE? file workflow would allow > >> iterative building of a commit message before committing > >> > > > > I absolutely despise --amend commits. It's the most annoying thing I have > > to do whenever I'm creating patches in a git clone. > > I do not think it is the appropriate place for the WebKit project to make judgments about personal preferences for local development. From my discussions with folks, it seems that the project is pretty evenly split on --amend
Re: [webkit-dev] ChangeLog Deprecation Plan
> Message: 2 > Date: Mon, 18 Apr 2022 10:33:03 -0700 > From: Ryosuke Niwa > To: Jonathan Bedard > Cc: WebKit Development > Subject: Re: [webkit-dev] ChangeLog Deprecation Plans > Message-ID: > > Content-Type: text/plain; charset="utf-8" > > On Mon, Apr 18, 2022 at 8:30 AM Jonathan Bedard via webkit-dev < > webkit-dev@lists.webkit.org> wrote: > >> As we migrate WebKit from Subversion to git, I would like to migrate the >> project away from ChangeLogs. The reason for this is that ChangeLogs make >> some of the features of git hard to use, namely, cherry-picking commits >> between branches requires conflict resolution every time. >> > > Isn't this something that can be easily resolved with a merge script? For simple cases, yes. But with something like a multi-commit revert or rebasing a feature onto a different branch, merge scripts stop working pretty quickly. It’s usually, It might be possible to write a merge script to handle these cases, but it’s by no means “easy” in the general case. It’s also worth noting that the cases that are more complicated also tend to be the ones with the most urgency attached to them. > > Rotating ChangeLogs is also moderately difficult in git with locked down >> commit access like our project has, only repository administers would, in >> practice, be able to rotate ChangeLogs. >> > > It seems like we can just automate this by introducing a change log > rotating bot, which has the same privilege as commit queue. Anything that starts with “we can just have a bot which…” is not a simple or robust solution. Bots need maintenance and intervention, and a bot with commit access has another set of issues. Repository admins occasionally rotating ChangeLogs is going to be less expensive than a bot doing it. > > So these two arguments seem rather weak. > > Lastly, ChangeLogs are uncommon in git based projects, so new contributors >> will find them difficult to manage. >> > > This might be the strongest argument in favor of deprecating change logs. > > 2) We need a way to comment on commit messages in review >> Current tooling sets the pull request description as the commit message, >> ?Quote Reply? kind of provides a way to inline comment, although it?s not >> the formal review UI >> *Proposal*: Tooling should support a ?COMMIT_MESSAGE? file in each pull >> request commit that becomes COMMIT_MESSAGE when a pull request is landed >> > > This needs to be a mandatory / automatic system, not opt-in. I want to > comment on commit messages as a reviewer. As a patch author, I don't care > whether it can be easily commented on or not. > > But if this is a required thing, then new contributors would have to learn > that this file is auto-generated or needs to be edited manually in some > cases so getting rid of change logs may not necessarily reduce the > cognitive load in comparison to keeping the status quo (i.e. keeping change > log files). We can make it automatic for folks using tooling (and in such a way that folks familiar with git won’t realize what has happened until they see their PR diff), “mandatory” is a bit tougher because we can’t prevent folks from filing a pull request without using tooling. At that point, we’re where we are today with PR template that encourages tool usage, explains how to craft a PR the ways tools do it and then reviewers acting as a gate-keeper. The last point is more about project policy than it is tooling. > > 3) Edit commit messages while creating a change, not just when committing >> the change >> The ?overwrite? workflow already sort of support this idea by using amend >> commits instead of appending commits to an existing branch >> *Proposal*: The above ?COMMIT_MESSAGE? file workflow would allow >> iterative building of a commit message before committing >> > > I absolutely despise --amend commits. It's the most annoying thing I have > to do whenever I'm creating patches in a git clone. I do not think it is the appropriate place for the WebKit project to make judgments about personal preferences for local development. From my discussions with folks, it seems that the project is pretty evenly split on --amend commits, with some contributors preferring them and others preferring to make multiple commits. My purpose in bringing up --amend commits is that one of the things I’ve heard folks find useful about ChangeLogs is the ability to iteratively build them, which is something that does have a parallel in raw git workflows. The COMMIT_MESSAGE approach, I believe, supports both --amend and amend workflows quite well. > > I like the COMMIT_MESSAGE and hooks proposals because they are opt-in. >> Contributors who wish to use native git tooling to contribute and interact >> with the project do not have to use either tool, but the tools are >> compatible enough with native git workflows that contributors who find >> editing and viewing commit messages primarily in a text editor >> > > I don't think
