Re: [webkit-dev] ChangeLog Deprecation Plans

2022-04-22 Thread Chris Dumez via webkit-dev
I spent some time discussing this offline with Yusuke to better understand his 
proposal.

It sounds like Yusuke’s proposal is:
1. Have a separate COMMIT_MESSAGE file as part of the PR
2. When the merge queue commits the PR, it uses that COMMIT_MESSAGE content as 
commit log message and then drops the file from the commit.

I understand that this approach is a bit more flexible for people who like to 
make multiple commits locally for the same PR. I personally always have a 
single commit per PR that I keep amending but I think it is good to be flexible.
It also makes reviewing the changelog message a bit easier on GitHub.

I support Yusuke’s proposal. It is a bit more flexible and it still means that 
separate full/history ChangeLog files would be a thing of the past.

--
 Chris Dumez




> On Apr 22, 2022, at 2:10 PM, Chris Dumez via webkit-dev 
>  wrote:
> 
> I am strongly in favor of dropping the ChangeLog files and relying on the GIT 
> commit message instead. Not having to update ChangeLog files was actually a 
> big motivator for me to switch to GitHub, as I thought until now we didn’t 
> have to update ChangeLog files when switching Github PRs.
> 
> With the provided GIT commit hook, the changelog format is identical to what 
> we had in the ChangeLog files anyway. I don’t understand (yet) the need for 
> having the same message in two separate location.
> 
> Git commit logs don’t roll over (like ChangeLog files do), they are 
> searchable, they have the same format (you CAN add inline comments on a 
> per-file basis for e.g.). They are also easily modifiable from the GitHub 
> interface.
> 
> I will also note that ChangeLog files are a frequent source of conflicts when 
> using GIT. Yes, we do have a resolve-ChangeLogs script that’s supposed to 
> help with that. However, please note that this script hasn’t work reliably 
> for quite a while (at least for many of us at Apple with very recent SDKs).
> ChangeLog entries no longer show on top when rebasing, meaning I have to move 
> them back on top manually. Worse, if I fail to notice and use `webkit-patch 
> upload`, it will upload to the wrong bugzilla bug.
> 
> If people really still want to maintain separate ChangeLog files, I am hoping 
> this can be generated from my commit messages and done automatically for me 
> upon committing. I really don’t want that as part of my patch/PR.
> 
> --
>  Chris Dumez
> 
> 
> 
> 
>> On Apr 19, 2022, at 11:00 AM, Geoffrey Garen via webkit-dev 
>> mailto:webkit-dev@lists.webkit.org>> wrote:
>> 
 Commit message is tied to a commit, so editing commit without breaking 
 commit-message is hard (how to revert one change from one commit without 
 rewriting commit message? It requires some git hack). Separate independent 
 COMMIT_MESSAGE file can solve this problem and makes local development 
 easy.
>>> 
>>> Developers who are used to git -- which nowadays is pretty much everyone 
>>> except WebKit devs -- are also used to rewriting commit messages.
>> 
>> I think it’s important for WebKit's git transition to take consideration of 
>> WebKit developers and WebKit workflows. I hope we can agree on this premise 
>> as we discuss various options for commit messages.
>> 
>> Thanks,
>> Geoff
>> ___
>> 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

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] ChangeLog Deprecation Plans

2022-04-22 Thread Chris Dumez via webkit-dev
I am strongly in favor of dropping the ChangeLog files and relying on the GIT 
commit message instead. Not having to update ChangeLog files was actually a big 
motivator for me to switch to GitHub, as I thought until now we didn’t have to 
update ChangeLog files when switching Github PRs.

With the provided GIT commit hook, the changelog format is identical to what we 
had in the ChangeLog files anyway. I don’t understand (yet) the need for having 
the same message in two separate location.

Git commit logs don’t roll over (like ChangeLog files do), they are searchable, 
they have the same format (you CAN add inline comments on a per-file basis for 
e.g.). They are also easily modifiable from the GitHub interface.

I will also note that ChangeLog files are a frequent source of conflicts when 
using GIT. Yes, we do have a resolve-ChangeLogs script that’s supposed to help 
with that. However, please note that this script hasn’t work reliably for quite 
a while (at least for many of us at Apple with very recent SDKs).
ChangeLog entries no longer show on top when rebasing, meaning I have to move 
them back on top manually. Worse, if I fail to notice and use `webkit-patch 
upload`, it will upload to the wrong bugzilla bug.

If people really still want to maintain separate ChangeLog files, I am hoping 
this can be generated from my commit messages and done automatically for me 
upon committing. I really don’t want that as part of my patch/PR.

--
 Chris Dumez




> On Apr 19, 2022, at 11:00 AM, Geoffrey Garen via webkit-dev 
>  wrote:
> 
>>> Commit message is tied to a commit, so editing commit without breaking 
>>> commit-message is hard (how to revert one change from one commit without 
>>> rewriting commit message? It requires some git hack). Separate independent 
>>> COMMIT_MESSAGE file can solve this problem and makes local development easy.
>> 
>> Developers who are used to git -- which nowadays is pretty much everyone 
>> except WebKit devs -- are also used to rewriting commit messages.
> 
> I think it’s important for WebKit's git transition to take consideration of 
> WebKit developers and WebKit workflows. I hope we can agree on this premise 
> as we discuss various options for commit messages.
> 
> Thanks,
> Geoff
> ___
> 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


Re: [webkit-dev] ChangeLog Deprecation Plans

2022-04-19 Thread Geoffrey Garen via webkit-dev
>> Commit message is tied to a commit, so editing commit without breaking 
>> commit-message is hard (how to revert one change from one commit without 
>> rewriting commit message? It requires some git hack). Separate independent 
>> COMMIT_MESSAGE file can solve this problem and makes local development easy.
> 
> Developers who are used to git -- which nowadays is pretty much everyone 
> except WebKit devs -- are also used to rewriting commit messages.

I think it’s important for WebKit's git transition to take consideration of 
WebKit developers and WebKit workflows. I hope we can agree on this premise as 
we discuss various options for commit messages.

Thanks,
Geoff
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] ChangeLog Deprecation Plans

2022-04-18 Thread Yusuke Suzuki via webkit-dev


> On Apr 18, 2022, at 5:38 PM, Michael Catanzaro  wrote:
> 
> 
> On Mon, Apr 18 2022 at 02:55:08 PM -0700, Yusuke Suzuki  
> wrote:
>> I think this is important. We are using commit message / ChangeLog as a 
>> document tied to the change, and we are writing very detailed description to 
>> make the intention / design of the change clear and making it as a good 
>> document when we read the change via git-blame, bisect, using that header, 
>> investigating how it works etc.
>> To make / keep this commit message / ChangeLog helpful, we need review, and 
>> I think reviewing of these messages is critical to keep usefulness of them.
> 
> I agree with all of the above. Actually, I'd suggest that the transition to 
> git is a good opportunity to become a little stricter with commit message 
> format than we historically have been. Except for trivial/obvious changes, 
> more detail is better.
> 
> However, we don't need ChangeLog files in the repo or inline review comments 
> to do this. I'm sure we can make do with the normal GitHub review UI.

GitHub (probably, Phabricator too) does not have UI to review commit message.
Also, showing commit message in an attached diff is pretty much similar to how 
Gerrit etc. shows the commit message for review. (Gerrit shows "Commit Message" 
diff).

> 
>> I think COMMIT_MESSAGE proposal has various good benefits.
>> 1. We can review commit mesasges.
>> 2. In local development, we can commit expected COMMIT_MESSAGE file directly 
>> in our tree. We can eventually add more and more detailed information to 
>> this file while local development continues, and we can also revert 
>> COMMIT_MESSAGE change since it can be commited in the local branch.
> 
> This has few advantages over using 'git commit --amend' or 'git rebase -i'. 
> It would also be a custom, WebKit-specific part of our workflow. This is a 
> good opportunity to remove as many WebKit-isms from our contribution process 
> as possible, to make it easier for people who are not familiar with the 
> project to contribute more easily. I suggest we at least try to do things 
> like most other open source projects first, and only implement custom 
> solutions if that fails.

Many projects are using multiple commits in one PR. Some projects require 
squashed merge (e.g. CoreCLR 
https://github.com/dotnet/coreclr/blob/master/Documentation/project-docs/contributing-workflow.md#suggested-workflow,
 https://github.blog/2016-04-01-squash-your-commits/ 
), and some projects just 
merge all of them. Because of the importance of performance / regression 
bisection, we must require landing a PR as one commit, so we need a way to 
squash commits.
If we would like to sane commit message for merged commits, we need to chose 
COMMIT_MESSAGE-like separated metadata for message or squashing manually.
Having separated metadata is much easier for development than manually 
squashing commits and messages because we can keep the final commit message 
safely even while changing history / code in this branch, and it was how 
webkit-patch worked.

> 
>> Commit message is tied to a commit, so editing commit without breaking 
>> commit-message is hard (how to revert one change from one commit without 
>> rewriting commit message? It requires some git hack). Separate independent 
>> COMMIT_MESSAGE file can solve this problem and makes local development easy.
> 
> Developers who are used to git -- which nowadays is pretty much everyone 
> except WebKit devs -- are also used to rewriting commit messages.
> 
> Developers are NOT used to writing commit messages in a file named 
> COMMIT_MESSAGE.

No. Various OSS projects require writing commit message separately from 
git-commit's message.
Chromium requires writing separate commit message data to upload a patch to 
Gerrit (git-cl).
And probably LLVM is too. (Using arc tool to upload a change to Phabricator).
They are conceptually the same to COMMIT_MESSAGE: managing non git-commit 
message for upstreaming.

Also, I'm fine if we can separately keep this message from git-commit. 
COMMIT_MESSAGE is one possible way.
But if we would like to have git-cl like tool which manages message separately 
from git-commit messages, then it is also fine. (but in that case, how to 
review this commit message is yet another problem).

> 
>> 3. This solves the problem how to squash multiple commits in one PR. 
>> Merge-queue can just look at this file and use this as a commit message when 
>> squashing and landing. This means that, in a PR, we can push multiple small 
>> commits in our PR branch.
> 
> What is the advantage to doing this, though? It's best if the commits in your 
> PR match what you intend to land. The structure of commits is subject to 
> review in most open source projects. If the commit history is messy, we 
> should not approve the PR.
> 
> For now, we've agreed that for now each PR should land as one commit.

We do not need to repeatedly 

Re: [webkit-dev] ChangeLog Deprecation Plans

2022-04-18 Thread Ryosuke Niwa via webkit-dev
On Mon, Apr 18, 2022 at 18:50 Fujii Hironori via webkit-dev <
webkit-dev@lists.webkit.org> wrote:

>
> On Tue, Apr 19, 2022 at 6:55 AM Yusuke Suzuki via webkit-dev <
> webkit-dev@lists.webkit.org> wrote:
>
>> I think this is important. We are using commit message / ChangeLog as a
>> document tied to the change, and we are writing very detailed description
>> to make the intention / design of the change clear and making it as a good
>> document when we read the change via git-blame, bisect, using that header,
>> investigating how it works etc.
>> To make / keep this commit message / ChangeLog helpful, we need review,
>> and I think reviewing of these messages is critical to keep usefulness of
>> them.
>>
>
>
> I don't think commit messages and ChangeLogs are the best place for
> technical descriptions.
> We use them because we don't have a better place.
>
> libpas added the technical document in the repository in markdown.
>
> https://github.com/WebKit/WebKit/blob/main/Source/bmalloc/libpas/Documentation.md
>
> This makes it possible to change code and update the document in a single
> commit, and get reviewed.
> markdown is better than plain text. Updated documents are more useful than
> the historical wiki pages.
> It'd be nice if more documents are migrated into the repository.
>

Documentation gets obsolete relatively quickly. Code is the source of
truth. A lot of comments in WebKit are either outdated, incorrect, and/or
misleading. Change logs describe a change and thereby a specific point in
time of the repository and wouldn't carry the same issue.

It's also particularly useful to be able to see why a given line of code
was written. Was it an oversight? On purpose? Or perhaps the required
behavior different then and now? It's impractical and unproductive to add
such a comment on every line of our codebase, and that level of online
comment will add so much clutter to the codebase and would make it harder
to read the code.

- R. Niwa
-- 
- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] ChangeLog Deprecation Plans

2022-04-18 Thread Saam Barati via webkit-dev
I think we need both of these things, and I don't think they should be mutually 
exclusive. Having motivation/design/details/gotchas go into a "changelog" 
accompanying a commit is very useful when blaming code.

- Saam

> On Apr 18, 2022, at 6:49 PM, Fujii Hironori via webkit-dev 
>  wrote:
> 
> 
> On Tue, Apr 19, 2022 at 6:55 AM Yusuke Suzuki via webkit-dev 
> mailto:webkit-dev@lists.webkit.org>> wrote:
> I think this is important. We are using commit message / ChangeLog as a 
> document tied to the change, and we are writing very detailed description to 
> make the intention / design of the change clear and making it as a good 
> document when we read the change via git-blame, bisect, using that header, 
> investigating how it works etc.
> To make / keep this commit message / ChangeLog helpful, we need review, and I 
> think reviewing of these messages is critical to keep usefulness of them.
> 
> 
> I don't think commit messages and ChangeLogs are the best place for technical 
> descriptions.
> We use them because we don't have a better place.

> 
> libpas added the technical document in the repository in markdown.
> https://github.com/WebKit/WebKit/blob/main/Source/bmalloc/libpas/Documentation.md
>  
> 
> This makes it possible to change code and update the document in a single 
> commit, and get reviewed.
> markdown is better than plain text. Updated documents are more useful than 
> the historical wiki pages.
> It'd be nice if more documents are migrated into the repository.
>  
> ___
> 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


Re: [webkit-dev] ChangeLog Deprecation Plans

2022-04-18 Thread Fujii Hironori via webkit-dev
On Tue, Apr 19, 2022 at 6:55 AM Yusuke Suzuki via webkit-dev <
webkit-dev@lists.webkit.org> wrote:

> I think this is important. We are using commit message / ChangeLog as a
> document tied to the change, and we are writing very detailed description
> to make the intention / design of the change clear and making it as a good
> document when we read the change via git-blame, bisect, using that header,
> investigating how it works etc.
> To make / keep this commit message / ChangeLog helpful, we need review,
> and I think reviewing of these messages is critical to keep usefulness of
> them.
>


I don't think commit messages and ChangeLogs are the best place for
technical descriptions.
We use them because we don't have a better place.

libpas added the technical document in the repository in markdown.
https://github.com/WebKit/WebKit/blob/main/Source/bmalloc/libpas/Documentation.md

This makes it possible to change code and update the document in a single
commit, and get reviewed.
markdown is better than plain text. Updated documents are more useful than
the historical wiki pages.
It'd be nice if more documents are migrated into the repository.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] ChangeLog Deprecation Plans

2022-04-18 Thread Michael Catanzaro via webkit-dev



On Mon, Apr 18 2022 at 02:55:08 PM -0700, Yusuke Suzuki 
 wrote:
I think this is important. We are using commit message / ChangeLog as 
a document tied to the change, and we are writing very detailed 
description to make the intention / design of the change clear and 
making it as a good document when we read the change via git-blame, 
bisect, using that header, investigating how it works etc.
To make / keep this commit message / ChangeLog helpful, we need 
review, and I think reviewing of these messages is critical to keep 
usefulness of them.


I agree with all of the above. Actually, I'd suggest that the 
transition to git is a good opportunity to become a little stricter 
with commit message format than we historically have been. Except for 
trivial/obvious changes, more detail is better.


However, we don't need ChangeLog files in the repo or inline review 
comments to do this. I'm sure we can make do with the normal GitHub 
review UI.



I think COMMIT_MESSAGE proposal has various good benefits.

1. We can review commit mesasges.
2. In local development, we can commit expected COMMIT_MESSAGE file 
directly in our tree. We can eventually add more and more detailed 
information to this file while local development continues, and we 
can also revert COMMIT_MESSAGE change since it can be commited in the 
local branch.


This has few advantages over using 'git commit --amend' or 'git rebase 
-i'. It would also be a custom, WebKit-specific part of our workflow. 
This is a good opportunity to remove as many WebKit-isms from our 
contribution process as possible, to make it easier for people who are 
not familiar with the project to contribute more easily. I suggest we 
at least try to do things like most other open source projects first, 
and only implement custom solutions if that fails.


Commit message is tied to a commit, so editing commit without 
breaking commit-message is hard (how to revert one change from one 
commit without rewriting commit message? It requires some git hack). 
Separate independent COMMIT_MESSAGE file can solve this problem and 
makes local development easy.


Developers who are used to git -- which nowadays is pretty much 
everyone except WebKit devs -- are also used to rewriting commit 
messages.


Developers are NOT used to writing commit messages in a file named 
COMMIT_MESSAGE.


3. This solves the problem how to squash multiple commits in one PR. 
Merge-queue can just look at this file and use this as a commit 
message when squashing and landing. This means that, in a PR, we can 
push multiple small commits in our PR branch.


What is the advantage to doing this, though? It's best if the commits 
in your PR match what you intend to land. The structure of commits is 
subject to review in most open source projects. If the commit history 
is messy, we should not approve the PR.


For now, we've agreed that for now each PR should land as one commit.


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] ChangeLog Deprecation Plans

2022-04-18 Thread Yusuke Suzuki via webkit-dev


> On Apr 18, 2022, at 12:51 PM, Michael Catanzaro via webkit-dev 
>  wrote:
> 
> On Mon, Apr 18 2022 at 08:30:04 AM -0700, Jonathan Bedard via webkit-dev 
>  wrote:
>> 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
> 
> Although it's inconvenient that we won't be able to leave inline comments on 
> commit messages anymore, is that really so serious a loss that it requires a 
> strange workaround? It just doesn't seem like a very big deal? We can copy 
> and paste and quote when we suggest changes in commit messages.

I think this is important. We are using commit message / ChangeLog as a 
document tied to the change, and we are writing very detailed description to 
make the intention / design of the change clear and making it as a good 
document when we read the change via git-blame, bisect, using that header, 
investigating how it works etc.
To make / keep this commit message / ChangeLog helpful, we need review, and I 
think reviewing of these messages is critical to keep usefulness of them.

I think COMMIT_MESSAGE proposal has various good benefits.

1. We can review commit mesasges.
2. In local development, we can commit expected COMMIT_MESSAGE file directly in 
our tree. We can eventually add more and more detailed information to this file 
while local development continues, and we can also revert COMMIT_MESSAGE change 
since it can be commited in the local branch. Commit message is tied to a 
commit, so editing commit without breaking commit-message is hard (how to 
revert one change from one commit without rewriting commit message? It requires 
some git hack). Separate independent COMMIT_MESSAGE file can solve this problem 
and makes local development easy.
3. This solves the problem how to squash multiple commits in one PR. 
Merge-queue can just look at this file and use this as a commit message when 
squashing and landing. This means that, in a PR, we can push multiple small 
commits in our PR branch. Merge-queue can get canonical COMMIT_MESSAGE when 
squashing, so keeping one-commit-per-one-PR in the upstream branch is easy. 
This allows normal git-based development workflow while keeping commit-message 
format canonical: creating a topic branch, committing many small changes to 
this branch, creating a PR, and PR gets merged into the upstream as a single 
commit.

-Yusuke

> 
>>  Proposal: Have Tools/Scripts/git-webkit setup configure hooks in 
>> contributors local git repositories to lay down CommitMessages.history files 
>> on merge, checkout and commit which contain the last 5000 commit messages. 
>> We can put these in similar places to where ChangeLogs are today, although 
>> we would likely want them in fewer places because this will increase local 
>> compute time on many git operations. We could also make this a configurable 
>> setting so that engineers who are more comfortable with the raw command line 
>> tooling do not have to deal with slower git operations.
> 
> What's wrong with `git log`?
> 
> There are GUI apps that can visualize your git history if so desired, e.g. 
> GNOME has gitg.
> 
> Michael
> 
> 
> ___
> 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


Re: [webkit-dev] ChangeLog Deprecation Plans

2022-04-18 Thread Ryosuke Niwa via webkit-dev
On Mon, Apr 18, 2022 at 13:34 Ryosuke Niwa  wrote:

>
> On Mon, Apr 18, 2022 at 08:30 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. 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. Lastly, ChangeLogs are uncommon in
>> git based projects, so new contributors will find them difficult to manage.
>>
>> Currently, our git tooling makes very little effort to either support
>> ChangeLogs or to provide alternatives. I have listed bellow some of the
>> reasons I understand folks to like ChangeLogs along with possible git-based
>> solutions, if necessary.
>>
>> 1) Subversion commit messages are stored server side, local development
>> needs a copy
>> git doesn’t have this problem. We have a local record of commit messages
>> in every checkout.
>> 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
>> 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
>>
>
> There is another issue to add here. Right now, change log entry exists for
> many sub directories of the respiratory like Source/WebCore and
> LayoutTests. With a commit message, however, there could be only one per
> patch / PR.
>
> This makes it harder to review each part separately. For example, I'd like
> to focus on core changes first and then onto test changes, or vice versa
> depending on patches I'm reviewing.
>
> So ideally, this commit message field mirror where change log files
> currently exist such that "commit message" for each sub directory will show
> up separately.
>
> 4) Using git commands to view commit messages is hard
>> There don’t seem to be many projects which have a solution for this. In
>> practice, it seems that reducing the scope of messages shown by restricting
>> history to a specific directory or even file is one solution, another is
>> shorter commit messages
>> *Proposal*: Have Tools/Scripts/git-webkit setup configure hooks in
>> contributors local git repositories to lay down CommitMessages.history
>> files on merge, checkout and commit which contain the last 5000 commit
>> messages. We can put these in similar places to where ChangeLogs are today,
>> although we would likely want them in fewer places because this will
>> increase local compute time on many git operations. We could also make this
>> a configurable setting so that engineers who are more comfortable with the
>> raw command line tooling do not have to deal with slower git operations.
>>
>> 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
>>
>> Looking forward to the discussion,
>>
>> Jonathan Bedard
>> WebKit Continuous Integration
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>>
> --
> - R. Niwa
>
-- 
- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] ChangeLog Deprecation Plans

2022-04-18 Thread Michael Catanzaro via webkit-dev
On Mon, Apr 18 2022 at 08:30:04 AM -0700, Jonathan Bedard via 
webkit-dev  wrote:

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


Although it's inconvenient that we won't be able to leave inline 
comments on commit messages anymore, is that really so serious a loss 
that it requires a strange workaround? It just doesn't seem like a very 
big deal? We can copy and paste and quote when we suggest changes in 
commit messages.


	Proposal: Have Tools/Scripts/git-webkit setup configure hooks in 
contributors local git repositories to lay down 
CommitMessages.history files on merge, checkout and commit which 
contain the last 5000 commit messages. We can put these in similar 
places to where ChangeLogs are today, although we would likely want 
them in fewer places because this will increase local compute time on 
many git operations. We could also make this a configurable setting 
so that engineers who are more comfortable with the raw command line 
tooling do not have to deal with slower git operations.


What's wrong with `git log`?

There are GUI apps that can visualize your git history if so desired, 
e.g. GNOME has gitg.


Michael


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] ChangeLog Deprecation Plans

2022-04-18 Thread Ryosuke Niwa via webkit-dev
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?

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.

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).

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 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 COMMIT_MESSAGE can be opt-in for the aforementioned reasons.

- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


[webkit-dev] ChangeLog Deprecation Plans

2022-04-18 Thread Jonathan Bedard via webkit-dev
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. 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. Lastly, ChangeLogs are uncommon in git based projects, so new 
contributors will find them difficult to manage.

Currently, our git tooling makes very little effort to either support 
ChangeLogs or to provide alternatives. I have listed bellow some of the reasons 
I understand folks to like ChangeLogs along with possible git-based solutions, 
if necessary.

1) Subversion commit messages are stored server side, local development needs a 
copy
git doesn’t have this problem. We have a local record of commit 
messages in every checkout.
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
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
4) Using git commands to view commit messages is hard
There don’t seem to be many projects which have a solution for this. In 
practice, it seems that reducing the scope of messages shown by restricting 
history to a specific directory or even file is one solution, another is 
shorter commit messages
Proposal: Have Tools/Scripts/git-webkit setup configure hooks in 
contributors local git repositories to lay down CommitMessages.history files on 
merge, checkout and commit which contain the last 5000 commit messages. We can 
put these in similar places to where ChangeLogs are today, although we would 
likely want them in fewer places because this will increase local compute time 
on many git operations. We could also make this a configurable setting so that 
engineers who are more comfortable with the raw command line tooling do not 
have to deal with slower git operations.

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

Looking forward to the discussion,

Jonathan Bedard
WebKit Continuous Integration___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev