'Re: [webkit-dev] WebKit Transition to Git'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       webkit-dev
Subject:    Re: [webkit-dev] WebKit Transition to Git
From:       Maciej Stachowiak <mjs () apple ! com>
Date:       2020-10-13 19:37:06
Message-ID: 55C2A5BD-E680-4319-89FB-7D16FB552875 () apple ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]

> On Oct 3, 2020, at 9:42 AM, Tetsuharu OHZEKI <tetsuharu.ohzeki@gmail.com> wrote:
> 
> > > I think having to create an account on a website isn't the main thing
> > > preventing people to contribute anyway? It's more about having to use
> > > project-specific tools to prepare the patch for submission (in the case
> > > of WebKit, having to write the commit message in the Changelog file, for
> > > example).
> > 
> > It's about all those things. We've definitely heard of people
> > complaining or refusing to create a Bugzilla account to report bugs.
> > I've gotta say I'm very much concerned about getting rid of change
> > logs when we move to Git. We put a lot of useful information about
> > what was causing the bug, how we fixed it, and why we fixed the way we
> > did in a change log. I've seen a few projects which transitioned to
> > Git and somehow lost the rigor in maintaining an equally high quality
> > commit message, partly because most code review tools don't let you
> > add inline comments to commit messages.
> 
> I'd like to tell my feelings about a ChangeLog file other than
> perspective for code review.

I think we should come up with a way to have the same level of detail in commit \
messages as is currently present in ChangeLog entries.

One example: ChangeLog-style commit message is included in a file in the diff (so \
that the review tool can properly review it), and it would be dropped and converted \
to a commit entry at merge time.

> 
> As a newbie patch contributor, it's true that ChangeLog file is a
> *bit* tired when I update a patch.
> I felt repeatedly that it should be replaced by VCS' commit log to
> make it easier.
> 
> On the other hand, ChangeLog file also helps me many times when I dig
> into the history of WebKit
> because ChangeLog file contains what functions are removed in which
> commit with Bugzilla id
> and sometimes includes detailed reason for changeset.
> 
> Perhaps it may be just that I don't know the way, VCS (I think here is
> Git) has a powerful feature to trace when we added codes
> but it's hard to trace when we removed codes, and it force a tough
> work to me when I investigate why we introduced this line to fix a
> bug.
> By contrast, WebKit's ChangeLog file is helpful to make it easier to
> trace both of adding ones and removing codes.
> 
> --
> Tetsuharu OHZEKI
> tetsuharu.ohzeki@gmail.com <mailto:tetsuharu.ohzeki@gmail.com>
> 
> On Sat, Oct 3, 2020 at 7:17 PM Ryosuke Niwa <rniwa@webkit.org \
> <mailto:rniwa@webkit.org>> wrote:
> > 
> > On Sat, Oct 3, 2020 at 2:25 AM Adrien Destugues
> > <pulkomandy@pulkomandy.tk <mailto:pulkomandy@pulkomandy.tk>> wrote:
> > > 
> > > On Fri, Oct 02, 2020 at 07:05:21PM -0500, Michael Catanzaro wrote:
> > > > > I realize that Gerrit might not integrate at all with hosting the repo
> > > > > on Github, but has any thought been given to this aspect of the
> > > > > transition?
> > > > 
> > > > That sounds like it would be a significant barrier to contribution, and
> > > > frankly defeat the point of switching. If we have serious concerns with
> > > > GitHub's code review functionality (which, again, I'm not familiar with),
> > > > then we should just use GitLab and have everything in one place. (GitLab
> > > > accepts GitHub logins via OAuth, both on gitlab.com and self-hosted
> > > > instances, so the barrier to contributing remains low either way.)
> > > 
> > > Gerrit accepts GitHub and other OAuth providers as well, so that's not a
> > > problem. We have been using this for Haiku code reviews for a few years
> > > now, and interestingly we got some complaints from people who don't want
> > > to have a Github account (for various reasons) and won't use our code
> > > review tool because of that.
> > > 
> > > I think the integration referred to was rather in terms of having
> > > reviews synchronized between Gerrit and Github pull requests, which is
> > > also possible, but I think if the point is to use Github, it doesn't
> > > work this way: if your workflow is too different from the standard way
> > > to use Github, people will still be confused by it and it will still be
> > > a barrier to contribution.
> > 
> > But using Gerrit would make that situation any better either.
> > 
> > > I think having to create an account on a website isn't the main thing
> > > preventing people to contribute anyway? It's more about having to use
> > > project-specific tools to prepare the patch for submission (in the case
> > > of WebKit, having to write the commit message in the Changelog file, for
> > > example).
> > 
> > It's about all those things. We've definitely heard of people
> > complaining or refusing to create a Bugzilla account to report bugs.
> > I've gotta say I'm very much concerned about getting rid of change
> > logs when we move to Git. We put a lot of useful information about
> > what was causing the bug, how we fixed it, and why we fixed the way we
> > did in a change log. I've seen a few projects which transitioned to
> > Git and somehow lost the rigor in maintaining an equally high quality
> > commit message, partly because most code review tools don't let you
> > add inline comments to commit messages.
> > 
> > - R. Niwa
> > _______________________________________________
> > webkit-dev mailing list
> > webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
> > https://lists.webkit.org/mailman/listinfo/webkit-dev \
> > <https://lists.webkit.org/mailman/listinfo/webkit-dev>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
> https://lists.webkit.org/mailman/listinfo/webkit-dev \
> <https://lists.webkit.org/mailman/listinfo/webkit-dev>

[Attachment #5 (unknown)]

<html><head><meta http-equiv="Content-Type" content="text/html; \
charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: \
space; line-break: after-white-space;" class=""><br class=""><div><br \
class=""><blockquote type="cite" class=""><div class="">On Oct 3, 2020, at 9:42 AM, \
Tetsuharu OHZEKI &lt;<a href="mailto:tetsuharu.ohzeki@gmail.com" \
class="">tetsuharu.ohzeki@gmail.com</a>&gt; wrote:</div><br \
class="Apple-interchange-newline"><div class=""><blockquote type="cite" \
style="font-family: Helvetica; font-size: 12px; font-style: normal; \
font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: \
auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; \
widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; \
-webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote \
type="cite" class="">I think having to create an account on a website isn't the main \
thing<br class="">preventing people to contribute anyway? It's more about having to \
use<br class="">project-specific tools to prepare the patch for submission (in the \
case<br class="">of WebKit, having to write the commit message in the Changelog file, \
for<br class="">example).<br class=""></blockquote><br class="">It's about all those \
things. We've definitely heard of people<br class="">complaining or refusing to \
create a Bugzilla account to report bugs.<br class="">I've gotta say I'm very much \
concerned about getting rid of change<br class="">logs when we move to Git. We put a \
lot of useful information about<br class="">what was causing the bug, how we fixed \
it, and why we fixed the way we<br class="">did in a change log. I've seen a few \
projects which transitioned to<br class="">Git and somehow lost the rigor in \
maintaining an equally high quality<br class="">commit message, partly because most \
code review tools don't let you<br class="">add inline comments to commit \
messages.<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: \
Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; \
font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; \
text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span \
style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; \
font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: \
normal; text-align: start; text-indent: 0px; text-transform: none; white-space: \
normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; \
float: none; display: inline !important;" class="">I'd like to tell my feelings about \
a ChangeLog file other than</span><br style="caret-color: rgb(0, 0, 0); font-family: \
Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; \
font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; \
text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span \
style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; \
font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: \
normal; text-align: start; text-indent: 0px; text-transform: none; white-space: \
normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; \
float: none; display: inline !important;" class="">perspective for code \
review.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; \
font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; \
letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; \
white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; \
text-decoration: none;" class=""></div></blockquote><div><br class=""></div><div>I \
think we should come up with a way to have the same level of detail in commit \
messages as is currently present in ChangeLog entries.</div><div><br \
class=""></div><div>One example: ChangeLog-style commit message is included in a file \
in the diff (so that the review tool can properly review it), and it would be dropped \
and converted to a commit entry at merge time.</div><br class=""><blockquote \
type="cite" class=""><div class=""><br style="caret-color: rgb(0, 0, 0); font-family: \
Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; \
font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; \
text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span \
style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; \
font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: \
normal; text-align: start; text-indent: 0px; text-transform: none; white-space: \
normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; \
float: none; display: inline !important;" class="">As a newbie patch contributor, \
it's true that ChangeLog file is a</span><br style="caret-color: rgb(0, 0, 0); \
font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: \
normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: \
0px; text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span \
style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; \
font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: \
normal; text-align: start; text-indent: 0px; text-transform: none; white-space: \
normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; \
float: none; display: inline !important;" class="">*bit* tired when I update a \
patch.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: \
12px; font-style: normal; font-variant-caps: normal; font-weight: normal; \
letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; \
white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; \
text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: \
Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; \
font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; \
text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline \
!important;" class="">I felt repeatedly that it should be replaced by VCS' commit log \
to</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: \
12px; font-style: normal; font-variant-caps: normal; font-weight: normal; \
letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; \
white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; \
text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: \
Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; \
font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; \
text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline \
!important;" class="">make it easier.</span><br style="caret-color: rgb(0, 0, 0); \
font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: \
normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: \
0px; text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br \
style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; \
font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: \
normal; text-align: start; text-indent: 0px; text-transform: none; white-space: \
normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" \
class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: \
12px; font-style: normal; font-variant-caps: normal; font-weight: normal; \
letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; \
white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; \
text-decoration: none; float: none; display: inline !important;" class="">On the \
other hand, ChangeLog file also helps me many times when I dig</span><br \
style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; \
font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: \
normal; text-align: start; text-indent: 0px; text-transform: none; white-space: \
normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" \
class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: \
12px; font-style: normal; font-variant-caps: normal; font-weight: normal; \
letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; \
white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; \
text-decoration: none; float: none; display: inline !important;" class="">into the \
history of WebKit</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; \
font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; \
letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; \
white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; \
text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: \
Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; \
font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; \
text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline \
!important;" class="">because ChangeLog file contains what functions are removed in \
which</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: \
12px; font-style: normal; font-variant-caps: normal; font-weight: normal; \
letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; \
white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; \
text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: \
Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; \
font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; \
text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline \
!important;" class="">commit with Bugzilla id</span><br style="caret-color: rgb(0, 0, \
0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: \
normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: \
0px; text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span \
style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; \
font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: \
normal; text-align: start; text-indent: 0px; text-transform: none; white-space: \
normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; \
float: none; display: inline !important;" class="">and sometimes includes detailed \
reason for changeset.</span><br style="caret-color: rgb(0, 0, 0); font-family: \
Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; \
font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; \
text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br \
style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; \
font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: \
normal; text-align: start; text-indent: 0px; text-transform: none; white-space: \
normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" \
class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: \
12px; font-style: normal; font-variant-caps: normal; font-weight: normal; \
letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; \
white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; \
text-decoration: none; float: none; display: inline !important;" class="">Perhaps it \
may be just that I don't know the way, VCS (I think here is</span><br \
style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; \
font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: \
normal; text-align: start; text-indent: 0px; text-transform: none; white-space: \
normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" \
class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: \
12px; font-style: normal; font-variant-caps: normal; font-weight: normal; \
letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; \
white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; \
text-decoration: none; float: none; display: inline !important;" class="">Git) has a \
powerful feature to trace when we added codes</span><br style="caret-color: rgb(0, 0, \
0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: \
normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: \
0px; text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span \
style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; \
font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: \
normal; text-align: start; text-indent: 0px; text-transform: none; white-space: \
normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; \
float: none; display: inline !important;" class="">but it's hard to trace when we \
removed codes, and it force a tough</span><br style="caret-color: rgb(0, 0, 0); \
font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: \
normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: \
0px; text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span \
style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; \
font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: \
normal; text-align: start; text-indent: 0px; text-transform: none; white-space: \
normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; \
float: none; display: inline !important;" class="">work to me when I investigate why \
we introduced this line to fix a</span><br style="caret-color: rgb(0, 0, 0); \
font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: \
normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: \
0px; text-transform: none; white-space: normal; word-spacing: 0px; \
-webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span \
style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; \
font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: \
normal; text-align: start; text-indent: 0px; text-transform: none; white-space: \
normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; \
float: none; display: inline !important;" class="">bug.</span><br style="caret-color: \
rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; \

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

[prev in list] [next in list] [prev in thread] [next in thread] 