subject:"\[webkit\-dev\] WebKit Transition to Git"

On Tue, Oct 13, 2020 at 4:12 PM Konstantin Tokarev  wrote:
>
>
>
> 14.10.2020, 02:01, "Ryosuke Niwa" :
> > On Tue, Oct 13, 2020 at 3:53 PM Konstantin Tokarev  
> > wrote:
> >>  14.10.2020, 01:45, "Ryosuke Niwa" :
> >>  > On Tue, Oct 13, 2020 at 3:40 PM Konstantin Tokarev  
> >> wrote:
> >>  >> 14.10.2020, 01:30, "Ryosuke Niwa" :
> >>  >> > On Tue, Oct 13, 2020 at 2:37 PM Konstantin Tokarev 
> >>  wrote:
> >>  >> >> 13.10.2020, 22:33, "Maciej Stachowiak" :
> >>  >> >> >> On Oct 2, 2020, at 10:59 AM, Michael Catanzaro 
> >>  wrote:
> >>  >> >> >>
> >>  >> >> >> On Fri, Oct 2, 2020 at 6:36 pm, Philippe Normand 
> >>  wrote:
> >>  >> >> >>> Would you also consider preventing merge commits in order to 
> >> keep a
> >>  >> >> >>> clean mainline branch?
> >>  >> >> >>
> >>  >> >> >> Big +1 to blocking merge commits. Merge commits in a huge 
> >> project like WebKit would make commit archaeology very frustrating. (I 
> >> assume this is implied by the monotonic commit identifiers proposal, but 
> >> it doesn't exactly say that.)
> >>  >> >> >
> >>  >> >> > I’m assuming your objection is to regular merges, but how do you 
> >> feel about squash merges? Or do you think all PRs should be landed by 
> >> rebasing?
> >>  >> >>
> >>  >> >> I'm not Michael but will add my 2 dollars anyway :)
> >>  >> >>
> >>  >> >> In these two approaches commits inside PR have different meaning, 
> >> and workflow is different.
> >>  >> >>
> >>  >> >> Below I use a term "atomic change" to describe minimal code change 
> >> which is a self-contained work unit with following properties:
> >>  >> >> * It implements well-defined task which can be summarized as a 
> >> short English sentence (typical soft limit is 60 characters)
> >>  >> >> * It doesn't introduce defects (e.g. bugs, compilation breakages, 
> >> style errors, typos) which were discovered during review process
> >>  >> >> * It doesn't include any code changes unrelated to main topic. This 
> >> separation is sometimes subjective, but it's usually recommended to split 
> >> refactoring and implementation of feature based on that, bug fix and new 
> >> feature, big style change and fix or feature.
> >>  >> >>
> >>  >> >> AFAIU our current review process has similar requirements to 
> >> patches submitted to Bugzilla, though sometimes patches include unrelated 
> >> changes. This can be justified by weakness of webkit-patch/Bugzilla 
> >> tooling which has no support for patch series, and by fact that SVN 
> >> doesn't support keeping local patch series at all.
> >>  >> >>
> >>  >> >> 1. Workflow 1 - "Squash merge" policy
> >>  >> >>
> >>  >> >> * Whole PR is considered to be a single atomic change of WebKit 
> >> source tree. If work is supposed to be landed as a series of changes which 
> >> depend on each other (e.g. refactoring and feature based on it, or 
> >> individual separate features touching same parts of code), each change 
> >> needs a separate PR, and, as a consequence, only one of them can be 
> >> efficiently reviewed at the moment of time
> >>  >> >> * Commits in PR represent review iterations or intermediate 
> >> implementation progress
> >>  >> >> * Reviewers' comments are addressed by pushing new commits without 
> >> rewriting history, which works around GitHub's lack of "commit revisions". 
> >> Also this workflow has lower entry barrier for people who haven't mastered 
> >> git yet, as it requires only "git commit" and "git push" without rebases.
> >>  >> >>
> >>  >> >> 2. Workflow 2 - "Rebase" ("cherry-pick")) or "Merge" policy
> >>  >> >>
> >>  >> >> * PR is considered to be a series of atomic changes. If work 
> >> consists of several atomic changes, each commit represent an atomic change
> >>  >> >> * Review iterations are done by fixing commits in place and 
> >> reuploading entire series using force push (of course if review discovers 
> >> that substantial part of work is missing it can be added as a new atomic 
> >> commit to the series)
> >>  >> >> * It's possible to review each commit in the series separately
> >>  >> >> * Workflow requires developers to have more discipline and 
> >> experience with using git rebase for history rewriting. Entry barrier can 
> >> be lowered by providing step by step instructions like e.g. [1].
> >>  >> >
> >>  >> > I really dislike this workflow due to its inherent complexity. Having
> >>  >> > to use Git is enough of a burden already. I don't want to deal with 
> >> an
> >>  >> > extra layer of complexity to deal with.
> >>  >>
> >>  >> There is simplified version of workflow 2 when you have only one 
> >> commit in PR. In this case you can easily edit this single commit with gic 
> >> commit --amend or GUI tools to address review comments. At the same time 
> >> those who are more comfortable with git can use longer patch series.
> >>  >
> >>  > Except that reviewers would still have to review each commit
> >>  > separately, and the time comes to revert someone's patch, we still
> >>  > need to remember how to

Re: [webkit-dev] WebKit Transition to Git



14.10.2020, 02:01, "Ryosuke Niwa" :
> On Tue, Oct 13, 2020 at 3:53 PM Konstantin Tokarev  wrote:
>>  14.10.2020, 01:45, "Ryosuke Niwa" :
>>  > On Tue, Oct 13, 2020 at 3:40 PM Konstantin Tokarev  
>> wrote:
>>  >> 14.10.2020, 01:30, "Ryosuke Niwa" :
>>  >> > On Tue, Oct 13, 2020 at 2:37 PM Konstantin Tokarev  
>> wrote:
>>  >> >> 13.10.2020, 22:33, "Maciej Stachowiak" :
>>  >> >> >> On Oct 2, 2020, at 10:59 AM, Michael Catanzaro 
>>  wrote:
>>  >> >> >>
>>  >> >> >> On Fri, Oct 2, 2020 at 6:36 pm, Philippe Normand 
>>  wrote:
>>  >> >> >>> Would you also consider preventing merge commits in order to keep 
>> a
>>  >> >> >>> clean mainline branch?
>>  >> >> >>
>>  >> >> >> Big +1 to blocking merge commits. Merge commits in a huge project 
>> like WebKit would make commit archaeology very frustrating. (I assume this 
>> is implied by the monotonic commit identifiers proposal, but it doesn't 
>> exactly say that.)
>>  >> >> >
>>  >> >> > I’m assuming your objection is to regular merges, but how do you 
>> feel about squash merges? Or do you think all PRs should be landed by 
>> rebasing?
>>  >> >>
>>  >> >> I'm not Michael but will add my 2 dollars anyway :)
>>  >> >>
>>  >> >> In these two approaches commits inside PR have different meaning, and 
>> workflow is different.
>>  >> >>
>>  >> >> Below I use a term "atomic change" to describe minimal code change 
>> which is a self-contained work unit with following properties:
>>  >> >> * It implements well-defined task which can be summarized as a short 
>> English sentence (typical soft limit is 60 characters)
>>  >> >> * It doesn't introduce defects (e.g. bugs, compilation breakages, 
>> style errors, typos) which were discovered during review process
>>  >> >> * It doesn't include any code changes unrelated to main topic. This 
>> separation is sometimes subjective, but it's usually recommended to split 
>> refactoring and implementation of feature based on that, bug fix and new 
>> feature, big style change and fix or feature.
>>  >> >>
>>  >> >> AFAIU our current review process has similar requirements to patches 
>> submitted to Bugzilla, though sometimes patches include unrelated changes. 
>> This can be justified by weakness of webkit-patch/Bugzilla tooling which has 
>> no support for patch series, and by fact that SVN doesn't support keeping 
>> local patch series at all.
>>  >> >>
>>  >> >> 1. Workflow 1 - "Squash merge" policy
>>  >> >>
>>  >> >> * Whole PR is considered to be a single atomic change of WebKit 
>> source tree. If work is supposed to be landed as a series of changes which 
>> depend on each other (e.g. refactoring and feature based on it, or 
>> individual separate features touching same parts of code), each change needs 
>> a separate PR, and, as a consequence, only one of them can be efficiently 
>> reviewed at the moment of time
>>  >> >> * Commits in PR represent review iterations or intermediate 
>> implementation progress
>>  >> >> * Reviewers' comments are addressed by pushing new commits without 
>> rewriting history, which works around GitHub's lack of "commit revisions". 
>> Also this workflow has lower entry barrier for people who haven't mastered 
>> git yet, as it requires only "git commit" and "git push" without rebases.
>>  >> >>
>>  >> >> 2. Workflow 2 - "Rebase" ("cherry-pick")) or "Merge" policy
>>  >> >>
>>  >> >> * PR is considered to be a series of atomic changes. If work consists 
>> of several atomic changes, each commit represent an atomic change
>>  >> >> * Review iterations are done by fixing commits in place and 
>> reuploading entire series using force push (of course if review discovers 
>> that substantial part of work is missing it can be added as a new atomic 
>> commit to the series)
>>  >> >> * It's possible to review each commit in the series separately
>>  >> >> * Workflow requires developers to have more discipline and experience 
>> with using git rebase for history rewriting. Entry barrier can be lowered by 
>> providing step by step instructions like e.g. [1].
>>  >> >
>>  >> > I really dislike this workflow due to its inherent complexity. Having
>>  >> > to use Git is enough of a burden already. I don't want to deal with an
>>  >> > extra layer of complexity to deal with.
>>  >>
>>  >> There is simplified version of workflow 2 when you have only one commit 
>> in PR. In this case you can easily edit this single commit with gic commit 
>> --amend or GUI tools to address review comments. At the same time those who 
>> are more comfortable with git can use longer patch series.
>>  >
>>  > Except that reviewers would still have to review each commit
>>  > separately, and the time comes to revert someone's patch, we still
>>  > need to remember how to revert a sequence of commits that belong to a
>>  > single PR.
>>
>>  Workflow 2 assumes that you forget about PR after it was merged and operate
>>  on its commits as equal parts of history.
>>
>>  In this sequence of commits each one can be reverted

Re: [webkit-dev] WebKit Transition to Git

On Tue, Oct 13, 2020 at 3:53 PM Konstantin Tokarev  wrote:
>
>
> 14.10.2020, 01:45, "Ryosuke Niwa" :
> > On Tue, Oct 13, 2020 at 3:40 PM Konstantin Tokarev  
> > wrote:
> >>  14.10.2020, 01:30, "Ryosuke Niwa" :
> >>  > On Tue, Oct 13, 2020 at 2:37 PM Konstantin Tokarev  
> >> wrote:
> >>  >> 13.10.2020, 22:33, "Maciej Stachowiak" :
> >>  >> >> On Oct 2, 2020, at 10:59 AM, Michael Catanzaro 
> >>  wrote:
> >>  >> >>
> >>  >> >> On Fri, Oct 2, 2020 at 6:36 pm, Philippe Normand  
> >> wrote:
> >>  >> >>> Would you also consider preventing merge commits in order to keep a
> >>  >> >>> clean mainline branch?
> >>  >> >>
> >>  >> >> Big +1 to blocking merge commits. Merge commits in a huge project 
> >> like WebKit would make commit archaeology very frustrating. (I assume this 
> >> is implied by the monotonic commit identifiers proposal, but it doesn't 
> >> exactly say that.)
> >>  >> >
> >>  >> > I’m assuming your objection is to regular merges, but how do you 
> >> feel about squash merges? Or do you think all PRs should be landed by 
> >> rebasing?
> >>  >>
> >>  >> I'm not Michael but will add my 2 dollars anyway :)
> >>  >>
> >>  >> In these two approaches commits inside PR have different meaning, and 
> >> workflow is different.
> >>  >>
> >>  >> Below I use a term "atomic change" to describe minimal code change 
> >> which is a self-contained work unit with following properties:
> >>  >> * It implements well-defined task which can be summarized as a short 
> >> English sentence (typical soft limit is 60 characters)
> >>  >> * It doesn't introduce defects (e.g. bugs, compilation breakages, 
> >> style errors, typos) which were discovered during review process
> >>  >> * It doesn't include any code changes unrelated to main topic. This 
> >> separation is sometimes subjective, but it's usually recommended to split 
> >> refactoring and implementation of feature based on that, bug fix and new 
> >> feature, big style change and fix or feature.
> >>  >>
> >>  >> AFAIU our current review process has similar requirements to patches 
> >> submitted to Bugzilla, though sometimes patches include unrelated changes. 
> >> This can be justified by weakness of webkit-patch/Bugzilla tooling which 
> >> has no support for patch series, and by fact that SVN doesn't support 
> >> keeping local patch series at all.
> >>  >>
> >>  >> 1. Workflow 1 - "Squash merge" policy
> >>  >>
> >>  >> * Whole PR is considered to be a single atomic change of WebKit source 
> >> tree. If work is supposed to be landed as a series of changes which depend 
> >> on each other (e.g. refactoring and feature based on it, or individual 
> >> separate features touching same parts of code), each change needs a 
> >> separate PR, and, as a consequence, only one of them can be efficiently 
> >> reviewed at the moment of time
> >>  >> * Commits in PR represent review iterations or intermediate 
> >> implementation progress
> >>  >> * Reviewers' comments are addressed by pushing new commits without 
> >> rewriting history, which works around GitHub's lack of "commit revisions". 
> >> Also this workflow has lower entry barrier for people who haven't mastered 
> >> git yet, as it requires only "git commit" and "git push" without rebases.
> >>  >>
> >>  >> 2. Workflow 2 - "Rebase" ("cherry-pick")) or "Merge" policy
> >>  >>
> >>  >> * PR is considered to be a series of atomic changes. If work consists 
> >> of several atomic changes, each commit represent an atomic change
> >>  >> * Review iterations are done by fixing commits in place and 
> >> reuploading entire series using force push (of course if review discovers 
> >> that substantial part of work is missing it can be added as a new atomic 
> >> commit to the series)
> >>  >> * It's possible to review each commit in the series separately
> >>  >> * Workflow requires developers to have more discipline and experience 
> >> with using git rebase for history rewriting. Entry barrier can be lowered 
> >> by providing step by step instructions like e.g. [1].
> >>  >
> >>  > I really dislike this workflow due to its inherent complexity. Having
> >>  > to use Git is enough of a burden already. I don't want to deal with an
> >>  > extra layer of complexity to deal with.
> >>
> >>  There is simplified version of workflow 2 when you have only one commit 
> >> in PR. In this case you can easily edit this single commit with gic commit 
> >> --amend or GUI tools to address review comments. At the same time those 
> >> who are more comfortable with git can use longer patch series.
> >
> > Except that reviewers would still have to review each commit
> > separately, and the time comes to revert someone's patch, we still
> > need to remember how to revert a sequence of commits that belong to a
> > single PR.
>
> Workflow 2 assumes that you forget about PR after it was merged and operate
> on its commits as equal parts of history.
>
> In this sequence of commits each one can be reverted on their own merits,
> like s

Re: [webkit-dev] WebKit Transition to Git

On Tue, Oct 13, 2020 at 3:19 PM Michael Catanzaro  wrote:
>
> Detailed descriptions are very important. I don't think function-level
> changelogs are; documenting changes in individual functions is
> generally busywork to say what you can plainly see by just looking at
> the diff.

They certainly can be quite informative. e.g.
https://trac.webkit.org/changeset/268365/webkit/trunk/Source/WebCore/ChangeLog

It's true that you can certainly split each logical step into its own
commit but that seems like more of a busy work to me. I mostly use a
Subversion checkout to do my work, and even if I'm using a Git clone,
I normally wouldn't commit anything until the whole patch is written.
e.g. I wrote the entirety of https://trac.webkit.org/r268239 and
posted in one chunk other than a few WIP patches I had posted. Having
to go back & split that into multiple commits would be a total waste
of time.

> Regarding line-by-line commit review... well, it would be nice to have,
> of course.  But I don't think it's as important as you suggest.
> Problems with commit messages are usually general problems with the
> entire commit message rather than problems with a specific line of the
> commit message.

I disagree. I often have specific commentary on specific lines of
change logs like missing function-level comments or typos, or need
some elaboration on specific details.

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

Re: [webkit-dev] WebKit Transition to Git



14.10.2020, 01:45, "Ryosuke Niwa" :
> On Tue, Oct 13, 2020 at 3:40 PM Konstantin Tokarev  wrote:
>>  14.10.2020, 01:30, "Ryosuke Niwa" :
>>  > On Tue, Oct 13, 2020 at 2:37 PM Konstantin Tokarev  
>> wrote:
>>  >> 13.10.2020, 22:33, "Maciej Stachowiak" :
>>  >> >> On Oct 2, 2020, at 10:59 AM, Michael Catanzaro  
>> wrote:
>>  >> >>
>>  >> >> On Fri, Oct 2, 2020 at 6:36 pm, Philippe Normand  
>> wrote:
>>  >> >>> Would you also consider preventing merge commits in order to keep a
>>  >> >>> clean mainline branch?
>>  >> >>
>>  >> >> Big +1 to blocking merge commits. Merge commits in a huge project 
>> like WebKit would make commit archaeology very frustrating. (I assume this 
>> is implied by the monotonic commit identifiers proposal, but it doesn't 
>> exactly say that.)
>>  >> >
>>  >> > I’m assuming your objection is to regular merges, but how do you feel 
>> about squash merges? Or do you think all PRs should be landed by rebasing?
>>  >>
>>  >> I'm not Michael but will add my 2 dollars anyway :)
>>  >>
>>  >> In these two approaches commits inside PR have different meaning, and 
>> workflow is different.
>>  >>
>>  >> Below I use a term "atomic change" to describe minimal code change which 
>> is a self-contained work unit with following properties:
>>  >> * It implements well-defined task which can be summarized as a short 
>> English sentence (typical soft limit is 60 characters)
>>  >> * It doesn't introduce defects (e.g. bugs, compilation breakages, style 
>> errors, typos) which were discovered during review process
>>  >> * It doesn't include any code changes unrelated to main topic. This 
>> separation is sometimes subjective, but it's usually recommended to split 
>> refactoring and implementation of feature based on that, bug fix and new 
>> feature, big style change and fix or feature.
>>  >>
>>  >> AFAIU our current review process has similar requirements to patches 
>> submitted to Bugzilla, though sometimes patches include unrelated changes. 
>> This can be justified by weakness of webkit-patch/Bugzilla tooling which has 
>> no support for patch series, and by fact that SVN doesn't support keeping 
>> local patch series at all.
>>  >>
>>  >> 1. Workflow 1 - "Squash merge" policy
>>  >>
>>  >> * Whole PR is considered to be a single atomic change of WebKit source 
>> tree. If work is supposed to be landed as a series of changes which depend 
>> on each other (e.g. refactoring and feature based on it, or individual 
>> separate features touching same parts of code), each change needs a separate 
>> PR, and, as a consequence, only one of them can be efficiently reviewed at 
>> the moment of time
>>  >> * Commits in PR represent review iterations or intermediate 
>> implementation progress
>>  >> * Reviewers' comments are addressed by pushing new commits without 
>> rewriting history, which works around GitHub's lack of "commit revisions". 
>> Also this workflow has lower entry barrier for people who haven't mastered 
>> git yet, as it requires only "git commit" and "git push" without rebases.
>>  >>
>>  >> 2. Workflow 2 - "Rebase" ("cherry-pick")) or "Merge" policy
>>  >>
>>  >> * PR is considered to be a series of atomic changes. If work consists of 
>> several atomic changes, each commit represent an atomic change
>>  >> * Review iterations are done by fixing commits in place and reuploading 
>> entire series using force push (of course if review discovers that 
>> substantial part of work is missing it can be added as a new atomic commit 
>> to the series)
>>  >> * It's possible to review each commit in the series separately
>>  >> * Workflow requires developers to have more discipline and experience 
>> with using git rebase for history rewriting. Entry barrier can be lowered by 
>> providing step by step instructions like e.g. [1].
>>  >
>>  > I really dislike this workflow due to its inherent complexity. Having
>>  > to use Git is enough of a burden already. I don't want to deal with an
>>  > extra layer of complexity to deal with.
>>
>>  There is simplified version of workflow 2 when you have only one commit in 
>> PR. In this case you can easily edit this single commit with gic commit 
>> --amend or GUI tools to address review comments. At the same time those who 
>> are more comfortable with git can use longer patch series.
>
> Except that reviewers would still have to review each commit
> separately, and the time comes to revert someone's patch, we still
> need to remember how to revert a sequence of commits that belong to a
> single PR.

Workflow 2 assumes that you forget about PR after it was merged and operate
on its commits as equal parts of history.

In this sequence of commits each one can be reverted on their own merits,
like separate (but consequential) Bugzilla patches in current workflow.
Sometimes it's not possible to revert one patch without reverting a few others
or solving conflicts, but you rarely think about reverting a whole range of
patches unless it becomes reall

Re: [webkit-dev] WebKit Transition to Git

On Tue, Oct 13, 2020 at 3:40 PM Konstantin Tokarev  wrote:
>
> 14.10.2020, 01:30, "Ryosuke Niwa" :
> > On Tue, Oct 13, 2020 at 2:37 PM Konstantin Tokarev  
> > wrote:
> >>  13.10.2020, 22:33, "Maciej Stachowiak" :
> >>  >> On Oct 2, 2020, at 10:59 AM, Michael Catanzaro  
> >> wrote:
> >>  >>
> >>  >> On Fri, Oct 2, 2020 at 6:36 pm, Philippe Normand  
> >> wrote:
> >>  >>> Would you also consider preventing merge commits in order to keep a
> >>  >>> clean mainline branch?
> >>  >>
> >>  >> Big +1 to blocking merge commits. Merge commits in a huge project like 
> >> WebKit would make commit archaeology very frustrating. (I assume this is 
> >> implied by the monotonic commit identifiers proposal, but it doesn't 
> >> exactly say that.)
> >>  >
> >>  > I’m assuming your objection is to regular merges, but how do you feel 
> >> about squash merges? Or do you think all PRs should be landed by rebasing?
> >>
> >>  I'm not Michael but will add my 2 dollars anyway :)
> >>
> >>  In these two approaches commits inside PR have different meaning, and 
> >> workflow is different.
> >>
> >>  Below I use a term "atomic change" to describe minimal code change which 
> >> is a self-contained work unit with following properties:
> >>  * It implements well-defined task which can be summarized as a short 
> >> English sentence (typical soft limit is 60 characters)
> >>  * It doesn't introduce defects (e.g. bugs, compilation breakages, style 
> >> errors, typos) which were discovered during review process
> >>  * It doesn't include any code changes unrelated to main topic. This 
> >> separation is sometimes subjective, but it's usually recommended to split 
> >> refactoring and implementation of feature based on that, bug fix and new 
> >> feature, big style change and fix or feature.
> >>
> >>  AFAIU our current review process has similar requirements to patches 
> >> submitted to Bugzilla, though sometimes patches include unrelated changes. 
> >> This can be justified by weakness of webkit-patch/Bugzilla tooling which 
> >> has no support for patch series, and by fact that SVN doesn't support 
> >> keeping local patch series at all.
> >>
> >>  1. Workflow 1 - "Squash merge" policy
> >>
> >>  * Whole PR is considered to be a single atomic change of WebKit source 
> >> tree. If work is supposed to be landed as a series of changes which depend 
> >> on each other (e.g. refactoring and feature based on it, or individual 
> >> separate features touching same parts of code), each change needs a 
> >> separate PR, and, as a consequence, only one of them can be efficiently 
> >> reviewed at the moment of time
> >>  * Commits in PR represent review iterations or intermediate 
> >> implementation progress
> >>  * Reviewers' comments are addressed by pushing new commits without 
> >> rewriting history, which works around GitHub's lack of "commit revisions". 
> >> Also this workflow has lower entry barrier for people who haven't mastered 
> >> git yet, as it requires only "git commit" and "git push" without rebases.
> >>
> >>  2. Workflow 2 - "Rebase" ("cherry-pick")) or "Merge" policy
> >>
> >>  * PR is considered to be a series of atomic changes. If work consists of 
> >> several atomic changes, each commit represent an atomic change
> >>  * Review iterations are done by fixing commits in place and reuploading 
> >> entire series using force push (of course if review discovers that 
> >> substantial part of work is missing it can be added as a new atomic commit 
> >> to the series)
> >>  * It's possible to review each commit in the series separately
> >>  * Workflow requires developers to have more discipline and experience 
> >> with using git rebase for history rewriting. Entry barrier can be lowered 
> >> by providing step by step instructions like e.g. [1].
> >
> > I really dislike this workflow due to its inherent complexity. Having
> > to use Git is enough of a burden already. I don't want to deal with an
> > extra layer of complexity to deal with.
>
> There is simplified version of workflow 2 when you have only one commit in 
> PR. In this case you can easily edit this single commit with gic commit 
> --amend or GUI tools to address review comments. At the same time those who 
> are more comfortable with git can use longer patch series.

Except that reviewers would still have to review each commit
separately, and the time comes to revert someone's patch, we still
need to remember how to revert a sequence of commits that belong to a
single PR.

I don't feel comfortable accepting this level of new complexity into
our contribution process in addition to being forced to use Git &
GitHub.

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

Re: [webkit-dev] WebKit Transition to Git

On Tue, Oct 13, 2020 at 3:19 PM Michael Catanzaro  wrote:
>
> I suppose what I'm describing is Konstantin's Workflow 2, which is
> overwhelmingly popular.
>
> On Tue, Oct 13, 2020 at 2:19 pm, Ryosuke Niwa  wrote:
> > Not squashing only helps if each commit can stand on its own. At that
> > point, I'd suggest such a sequence of commits be made into multiple
> > PRs instead of multiple commits in a single PR, each of which requires
> > a separate code review.
>
> Commits are certainly expected to stand on their own (not introduce
> defects). If they can't, then they should be combined into another
> commit! Hence, we should not approve MRs if the MR contains commits
> that fail to meet our usual standards. Such commits should just fail
> code review. (But if you aren't willing to review MRs commit-by-commit,
> then indeed you'll never notice when such problems exist.)

Right, so this will be a problem unless we're gonna review each commit
separately. But if we're doing that, we might as well as create a
separate PR for each commit.

If we have 5-10 separate commits each of which make substantial
changes, we don't all of them to be landed / merged all at once into
the main branch even if they're logically related. Without
intermediate test runs & perf test results, we wouldn't be able to
pin-point what caused it, in which case, we'd be likely forced to
revert all of them anyway.

Furthermore, reverting a part of a sequence of commits coming from a
single PR would be very confusing just reverting just one of many
patches posted on Bugzilla would be confusing.

For all these reasons, I don't see much benefit in allowing multiple
commits from being merged from PR.

> If I have to open a separate MR for each change, though, I'm going to
> wind up combining multiple lightly-related changes into one big commit,
> because a new MR for every tiny cleanup I want to make requires effort.
> Such commits may be common in WebKit, but they would fail code review
> in most other open source projects. E.g. in
> https://trac.webkit.org/changeset/268394/webkit I snuck in a drive-by
> one-line fix that's unrelated to the rest of the commit. I would rarely
> dare to do that outside WebKit, knowing it would probably fail review,
> but we do it commonly in WebKit because we discourage multiple patches
> per bug and don't want to create new bugs for every small cleanup.

That seems totally okay. I land one line fix like that all the time though.

> > This to me is a show stopper. When I'm trying to bisect an issue,
> > etc..., the biggest obstacle I face is any intermediate revisions
> > where builds are broken or otherwise non-functional. I don't think we
> > should let anyone merge a commit into the main branch unless the
> > commit meets the same standards as a regular Bugzilla patch we land
> > today.
>
> I agree. But I would say that a MR with such history should fail
> review, and be rewritten to not suffer from these problems.

The problem is that you're presuming that all WebKit reviewers would
be able to and would be willing to do that review. I certainly do not
feel comfortable having to do that given how troublesome it has been
in WPT land.

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

Re: [webkit-dev] WebKit Transition to Git



14.10.2020, 01:30, "Ryosuke Niwa" :
> On Tue, Oct 13, 2020 at 2:37 PM Konstantin Tokarev  wrote:
>>  13.10.2020, 22:33, "Maciej Stachowiak" :
>>  >> On Oct 2, 2020, at 10:59 AM, Michael Catanzaro  
>> wrote:
>>  >>
>>  >> On Fri, Oct 2, 2020 at 6:36 pm, Philippe Normand  
>> wrote:
>>  >>> Would you also consider preventing merge commits in order to keep a
>>  >>> clean mainline branch?
>>  >>
>>  >> Big +1 to blocking merge commits. Merge commits in a huge project like 
>> WebKit would make commit archaeology very frustrating. (I assume this is 
>> implied by the monotonic commit identifiers proposal, but it doesn't exactly 
>> say that.)
>>  >
>>  > I’m assuming your objection is to regular merges, but how do you feel 
>> about squash merges? Or do you think all PRs should be landed by rebasing?
>>
>>  I'm not Michael but will add my 2 dollars anyway :)
>>
>>  In these two approaches commits inside PR have different meaning, and 
>> workflow is different.
>>
>>  Below I use a term "atomic change" to describe minimal code change which is 
>> a self-contained work unit with following properties:
>>  * It implements well-defined task which can be summarized as a short 
>> English sentence (typical soft limit is 60 characters)
>>  * It doesn't introduce defects (e.g. bugs, compilation breakages, style 
>> errors, typos) which were discovered during review process
>>  * It doesn't include any code changes unrelated to main topic. This 
>> separation is sometimes subjective, but it's usually recommended to split 
>> refactoring and implementation of feature based on that, bug fix and new 
>> feature, big style change and fix or feature.
>>
>>  AFAIU our current review process has similar requirements to patches 
>> submitted to Bugzilla, though sometimes patches include unrelated changes. 
>> This can be justified by weakness of webkit-patch/Bugzilla tooling which has 
>> no support for patch series, and by fact that SVN doesn't support keeping 
>> local patch series at all.
>>
>>  1. Workflow 1 - "Squash merge" policy
>>
>>  * Whole PR is considered to be a single atomic change of WebKit source 
>> tree. If work is supposed to be landed as a series of changes which depend 
>> on each other (e.g. refactoring and feature based on it, or individual 
>> separate features touching same parts of code), each change needs a separate 
>> PR, and, as a consequence, only one of them can be efficiently reviewed at 
>> the moment of time
>>  * Commits in PR represent review iterations or intermediate implementation 
>> progress
>>  * Reviewers' comments are addressed by pushing new commits without 
>> rewriting history, which works around GitHub's lack of "commit revisions". 
>> Also this workflow has lower entry barrier for people who haven't mastered 
>> git yet, as it requires only "git commit" and "git push" without rebases.
>>
>>  2. Workflow 2 - "Rebase" ("cherry-pick")) or "Merge" policy
>>
>>  * PR is considered to be a series of atomic changes. If work consists of 
>> several atomic changes, each commit represent an atomic change
>>  * Review iterations are done by fixing commits in place and reuploading 
>> entire series using force push (of course if review discovers that 
>> substantial part of work is missing it can be added as a new atomic commit 
>> to the series)
>>  * It's possible to review each commit in the series separately
>>  * Workflow requires developers to have more discipline and experience with 
>> using git rebase for history rewriting. Entry barrier can be lowered by 
>> providing step by step instructions like e.g. [1].
>
> I really dislike this workflow due to its inherent complexity. Having
> to use Git is enough of a burden already. I don't want to deal with an
> extra layer of complexity to deal with.

There is simplified version of workflow 2 when you have only one commit in PR. 
In this case you can easily edit this single commit with gic commit --amend or 
GUI tools to address review comments. At the same time those who are more 
comfortable with git can use longer patch series.


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

Re: [webkit-dev] WebKit Transition to Git

On Tue, Oct 13, 2020 at 2:37 PM Konstantin Tokarev  wrote:
>
>
> 13.10.2020, 22:33, "Maciej Stachowiak" :
> >>  On Oct 2, 2020, at 10:59 AM, Michael Catanzaro  
> >> wrote:
> >>
> >>  On Fri, Oct 2, 2020 at 6:36 pm, Philippe Normand  wrote:
> >>>  Would you also consider preventing merge commits in order to keep a
> >>>  clean mainline branch?
> >>
> >>  Big +1 to blocking merge commits. Merge commits in a huge project like 
> >> WebKit would make commit archaeology very frustrating. (I assume this is 
> >> implied by the monotonic commit identifiers proposal, but it doesn't 
> >> exactly say that.)
> >
> > I’m assuming your objection is to regular merges, but how do you feel about 
> > squash merges? Or do you think all PRs should be landed by rebasing?
>
> I'm not Michael but will add my 2 dollars anyway :)
>
> In these two approaches commits inside PR have different meaning, and 
> workflow is different.
>
> Below I use a term "atomic change" to describe minimal code change which is a 
> self-contained work unit with following properties:
> * It implements well-defined task which can be summarized as a short English 
> sentence (typical soft limit is 60 characters)
> * It doesn't introduce defects (e.g. bugs, compilation breakages, style 
> errors, typos) which were discovered during review process
> * It doesn't include any code changes unrelated to main topic. This 
> separation is sometimes subjective, but it's usually recommended to split 
> refactoring and implementation of feature based on that, bug fix and new 
> feature, big style change and fix or feature.
>
> AFAIU our current review process has similar requirements to patches 
> submitted to Bugzilla, though sometimes patches include unrelated changes. 
> This can be justified by weakness of webkit-patch/Bugzilla tooling which has 
> no support for patch series, and by fact that SVN doesn't support keeping 
> local patch series at all.
>
> 1. Workflow 1 - "Squash merge" policy
>
> * Whole PR is considered to be a single atomic change of WebKit source tree. 
> If work is supposed to be landed as a series of changes which depend on each 
> other (e.g. refactoring and feature based on it, or individual separate 
> features touching same parts of code), each change needs a separate PR, and, 
> as a consequence, only one of them can be efficiently reviewed at the moment 
> of time
> * Commits in PR represent review iterations or intermediate implementation 
> progress
> * Reviewers' comments are addressed by pushing new commits without rewriting 
> history, which works around GitHub's lack of "commit revisions". Also this 
> workflow has lower entry barrier for people who haven't mastered git yet, as 
> it requires only "git commit" and "git push" without rebases.
>
> 2. Workflow 2 - "Rebase" ("cherry-pick")) or "Merge" policy
>
> * PR is considered to be a series of atomic changes. If work consists of 
> several atomic changes, each commit represent an atomic change
> * Review iterations are done by fixing commits in place and reuploading 
> entire series using force push (of course if review discovers that 
> substantial part of work is missing it can be added as a new atomic commit to 
> the series)
> * It's possible to review each commit in the series separately
> * Workflow requires developers to have more discipline and experience with 
> using git rebase for history rewriting. Entry barrier can be lowered by 
> providing step by step instructions like e.g. [1].

I really dislike this workflow due to its inherent complexity. Having
to use Git is enough of a burden already. I don't want to deal with an
extra layer of complexity to deal with.

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

Re: [webkit-dev] WebKit Transition to Git

2020-10-13 Thread Michael Catanzaro




I suppose what I'm describing is Konstantin's Workflow 2, which is 
overwhelmingly popular.


On Tue, Oct 13, 2020 at 2:19 pm, Ryosuke Niwa  wrote:

Not squashing only helps if each commit can stand on its own. At that
point, I'd suggest such a sequence of commits be made into multiple
PRs instead of multiple commits in a single PR, each of which requires
a separate code review.


Commits are certainly expected to stand on their own (not introduce 
defects). If they can't, then they should be combined into another 
commit! Hence, we should not approve MRs if the MR contains commits 
that fail to meet our usual standards. Such commits should just fail 
code review. (But if you aren't willing to review MRs commit-by-commit, 
then indeed you'll never notice when such problems exist.)


If I have to open a separate MR for each change, though, I'm going to 
wind up combining multiple lightly-related changes into one big commit, 
because a new MR for every tiny cleanup I want to make requires effort. 
Such commits may be common in WebKit, but they would fail code review 
in most other open source projects. E.g. in 
https://trac.webkit.org/changeset/268394/webkit I snuck in a drive-by 
one-line fix that's unrelated to the rest of the commit. I would rarely 
dare to do that outside WebKit, knowing it would probably fail review, 
but we do it commonly in WebKit because we discourage multiple patches 
per bug and don't want to create new bugs for every small cleanup.



This to me is a show stopper. When I'm trying to bisect an issue,
etc..., the biggest obstacle I face is any intermediate revisions
where builds are broken or otherwise non-functional. I don't think we
should let anyone merge a commit into the main branch unless the
commit meets the same standards as a regular Bugzilla patch we land
today.


I agree. But I would say that a MR with such history should fail 
review, and be rewritten to not suffer from these problems.



I disagree. Detailed descriptions and function-level change logs are
exactly what I use to dig up all the code history and figure out
what's causing the bug and how to fix in numerous occasions. Not
having that would be a massive regression.

- R. Niwa


Detailed descriptions are very important. I don't think function-level 
changelogs are; documenting changes in individual functions is 
generally busywork to say what you can plainly see by just looking at 
the diff. I'll submit 
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1686/commits as an 
example of my preferred commit granularity and verbosity. (We can 
pretend it is not currently failing CI. ;) Here the commits are 
lightly-related and could perhaps be split into multiple MRs, but 
honestly that becomes annoying and unwieldy, especially as some of the 
commits depend on each other and need to land in a particular order, 
and since creating new branches and MRs (or bugs on Bugzilla) for each 
commit becomes annoying. So it's nicer to do it all in one. One of 
these commits actually makes changes that are undone by a subsequent 
commit, and is primarily there just so that I could write a commit 
message about it and show the history in two steps rather than one! 
History like that would be lost in Konstantin's Workflow 1. I don't 
think ChangeLog-style function-level detail would be helpful in any of 
them; in WebKit, I usually ignore that anyway. But all of the commits 
do have a detailed commit message, except for "fix typo" and "fix 
whitespace" (can't expect an essay for those).


Regarding line-by-line commit review... well, it would be nice to have, 
of course.  But I don't think it's as important as you suggest. 
Problems with commit messages are usually general problems with the 
entire commit message rather than problems with a specific line of the 
commit message. An exception is typos. It is harder to point out typos 
without a line-by-line review tool. But still, it's not too hard to 
tell somebody to fix a typo without being able to highlight the right 
line.


Michael


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

Re: [webkit-dev] WebKit Transition to Git

13.10.2020, 22:33, "Maciej Stachowiak" :
>>  On Oct 2, 2020, at 10:59 AM, Michael Catanzaro  wrote:
>>
>>  On Fri, Oct 2, 2020 at 6:36 pm, Philippe Normand  wrote:
>>>  Would you also consider preventing merge commits in order to keep a
>>>  clean mainline branch?
>>
>>  Big +1 to blocking merge commits. Merge commits in a huge project like 
>> WebKit would make commit archaeology very frustrating. (I assume this is 
>> implied by the monotonic commit identifiers proposal, but it doesn't exactly 
>> say that.)
>
> I’m assuming your objection is to regular merges, but how do you feel about 
> squash merges? Or do you think all PRs should be landed by rebasing?

I'm not Michael but will add my 2 dollars anyway :)

In these two approaches commits inside PR have different meaning, and workflow 
is different.

Below I use a term "atomic change" to describe minimal code change which is a 
self-contained work unit with following properties:
* It implements well-defined task which can be summarized as a short English 
sentence (typical soft limit is 60 characters)
* It doesn't introduce defects (e.g. bugs, compilation breakages, style errors, 
typos) which were discovered during review process
* It doesn't include any code changes unrelated to main topic. This separation 
is sometimes subjective, but it's usually recommended to split refactoring and 
implementation of feature based on that, bug fix and new feature, big style 
change and fix or feature.

AFAIU our current review process has similar requirements to patches submitted 
to Bugzilla, though sometimes patches include unrelated changes. This can be 
justified by weakness of webkit-patch/Bugzilla tooling which has no support for 
patch series, and by fact that SVN doesn't support keeping local patch series 
at all.

1. Workflow 1 - "Squash merge" policy

* Whole PR is considered to be a single atomic change of WebKit source tree. If 
work is supposed to be landed as a series of changes which depend on each other 
(e.g. refactoring and feature based on it, or individual separate features 
touching same parts of code), each change needs a separate PR, and, as a 
consequence, only one of them can be efficiently reviewed at the moment of time
* Commits in PR represent review iterations or intermediate implementation 
progress
* Reviewers' comments are addressed by pushing new commits without rewriting 
history, which works around GitHub's lack of "commit revisions". Also this 
workflow has lower entry barrier for people who haven't mastered git yet, as it 
requires only "git commit" and "git push" without rebases.

2. Workflow 2 - "Rebase" ("cherry-pick")) or "Merge" policy

* PR is considered to be a series of atomic changes. If work consists of 
several atomic changes, each commit represent an atomic change
* Review iterations are done by fixing commits in place and reuploading entire 
series using force push (of course if review discovers that substantial part of 
work is missing it can be added as a new atomic commit to the series)
* It's possible to review each commit in the series separately
* Workflow requires developers to have more discipline and experience with 
using git rebase for history rewriting. Entry barrier can be lowered by 
providing step by step instructions like e.g. [1]. 

Workflow 1 is more like what we have now with Bugzilla.

Workflow 2 is used by many projects which use git for a long time and value 
high-quality commit history. It's used e.g. by Linux kernel, or projects which 
use Gerrit as a review tool (Chromium, Android, Qt, etc). It advantages for 
developers (who can submit more work to review at the same time without risk of 
mixing up unrelated changes together), reviewers (it's easier to review atomic 
changes than those with too high or too low granularity), maintainers of stable 
branches (they can pick bug reports and avoid at least some unneeded 
refactorings, features and style improvements).

Workflow 1 is the most obvious way to use GitHub, because it doesn't make any 
hints about existence of workflow 2. However, many projects which use GitHub 
for code review and value high-quality history are using workflow 2. For 
example, see Ryosuke's example with whatwg/html repo.

Workflow 2 is facilitated by Gerrit, which doesn't require using force pushes 
and makes it immediately obvious for new user if they start adding new fixup 
commits instead of editing those being reviewed.

Workflow 2 can use both rebase and merge strategies, and merge isn't so evil in 
this case. However, using merge moves responsibility for solving conflicts from 
contributor to repository maintainers, which doesn't scale well. Projects which 
accept patches via mailing lists, like Linux kernel, imply that reviewed patch 
series must apply to the tip of the tree, so while there is no explicit 
"rebase" it is implied.

In conclusion, I recommend using 2 with "rebase" policy, but YMMV.

[1] https://wiki.qt.io/Gerrit_Introduction#Updating_a_Contri

Re: [webkit-dev] WebKit Transition to Git

On Tue, Oct 13, 2020 at 1:57 PM Michael Catanzaro  wrote:
>
> On Tue, Oct 13, 2020 at 12:32 pm, Maciej Stachowiak 
> wrote:
> > I’m assuming your objection is to regular merges, but how do you
> > feel about squash merges? Or do you think all PRs should be landed by
> > rebasing?
>
> If we want a linear history (we do), all PRs ultimately have to land as
> fast-forward merges, one way or the other. I think rebasing is the
> simplest way to accomplish that, but squashes work too if all your
> changes are sufficiently self-contained to land as one commit.
>
> I suggest leaving this up to the discretion of the developer and
> reviewer rather than mandating one way or the other, because there are
> advantages and disadvantages to both approaches. If your local commit
> history is a mess, sometimes squashing it all into one commit is an
> easy way to make it acceptable for upstream. If you hate rewriting
> history to make it look good, and your MR isn't too huge, a squash
> might be appropriate. But more often than not, I'd say that would
> result in a worse commit history.

FWIW, I think we should always squash all commits and have a single
change log entry / commit message for all changes.

In the case we want to rebase and keep multiple commits, then we
should have a change log / descriptive commit message that follows the
current change log format for each commit, rather than a short
description of each change.

> > My own preference would be to require squash merge, because it keeps
> > the history simple for the main branch, but does not risk putting
> > intermediate revisions which may work or even build on the main
> > branch.
>
> The disadvantage is that if you squash before merging, you encourage
> fewer, bigger commits that might be harder to read and potentially much
> less fun to discover at the end of a bisect.

Not squashing only helps if each commit can stand on its own. At that
point, I'd suggest such a sequence of commits be made into multiple
PRs instead of multiple commits in a single PR, each of which requires
a separate code review.

> On the other hand, if you don't squash, it's indeed possible that your
> commit history might be messy, e.g. intermediate commits might break
> tests fixed by subsequent commits, or intermediate commits might not
> build at all.

This to me is a show stopper. When I'm trying to bisect an issue,
etc..., the biggest obstacle I face is any intermediate revisions
where builds are broken or otherwise non-functional. I don't think we
should let anyone merge a commit into the main branch unless the
commit meets the same standards as a regular Bugzilla patch we land
today.

> Sometimes I see the
> opposite, where too many changes are bundled into one big commit. (This
> is more common in WebKit, since we normally try to have one patch per
> bug, and splitting changes into multiple bugs is inconvenient.) But
> usually the developer submitting a MR has found a good balance between
> the two extremes. Ultimately, what's most important is landing a clean
> commit history with reviewable commits. Again, the scope of commits is
> subject to review just like code changes are.

I often review test changes in WPT or spec changes in WHATWG / W3C,
and I often find a PR with multiple commits to be annoying to review.
I'd rather have each PR have a single large commit to review. In
practice, I'd never review commit by commit because I need to see the
whole picture to review what's happening.

> Instead, I suggest
> developers should aggressively use 'git add -p' and 'git rebase -i' to
> selectively commit, rewrite, and reorder history to look good before
> opening the MR. This isn't optional for most open source projects: if
> you propose an MR with ugly commit history, it won't be merged until
> fixed. For a typical MR of moderate complexity, I'll use 'git rebase
> -i' at least a couple times before the history is clean enough for a
> MR, and to make fixup commits disappear into the most-appropriate
> commit in the sequence.

I'm afraid that this will still result in random intermediary commits
being merged into the main branch because there is no mechanical
enforcement.

> Regarding commit messages, I don't understand why there's any
> expectation that they need to be checked into files in order to be
> detailed and complete. If a commit message doesn't adequately describe
> what it's fixing, then the reviewer should open a review thread to
> complain and request more detailed commit messages.

The problem is that I've seen multiple projects which moved to Git and
did this, and their commit log message's quality materially degraded.
If GitHub and other tools allowed inline comments being added for each
line of the commit message, I have less of concern. Github doesn't.

> If a change is very
> straightforward and obvious, sometimes a quick sentence might suffice,
> but usually a good commit message should be at least a paragraph or
> two, and often more. Function-level ChangeL

Re: [webkit-dev] WebKit Transition to Git

2020-10-13 Thread Michael Catanzaro

On Tue, Oct 13, 2020 at 12:32 pm, Maciej Stachowiak  
wrote:
I’m assuming your objection is to regular merges, but how do you 
feel about squash merges? Or do you think all PRs should be landed by 
rebasing?


If we want a linear history (we do), all PRs ultimately have to land as 
fast-forward merges, one way or the other. I think rebasing is the 
simplest way to accomplish that, but squashes work too if all your 
changes are sufficiently self-contained to land as one commit.


I suggest leaving this up to the discretion of the developer and 
reviewer rather than mandating one way or the other, because there are 
advantages and disadvantages to both approaches. If your local commit 
history is a mess, sometimes squashing it all into one commit is an 
easy way to make it acceptable for upstream. If you hate rewriting 
history to make it look good, and your MR isn't too huge, a squash 
might be appropriate. But more often than not, I'd say that would 
result in a worse commit history.


My own preference would be to require squash merge, because it keeps 
the history simple for the main branch, but does not risk putting 
intermediate revisions which may work or even build on the main 
branch.


The disadvantage is that if you squash before merging, you encourage 
fewer, bigger commits that might be harder to read and potentially much 
less fun to discover at the end of a bisect. E.g. consider a very 
common case: developer wants to fix a handful of separate but related 
bugs in a particular section of code. We could land all the fixes in 
one huge commit, but that's inevitably going to be harder to read, 
harder to deal with if it introduces a regression, and will certainly 
have a more confusing commit message (either because it gives less 
detail about the changes, or because it's very long describing multiple 
related changes that could have been separate commits). We could split 
it up into different MRs for the different commits, but that becomes 
annoying when the commits are related, and hard to keep the different 
MRs in order. So I don't normally squash (except when committing small 
fixup commits via the web UI), because the disadvantages generally 
outweigh the benefits.


On the other hand, if you don't squash, it's indeed possible that your 
commit history might be messy, e.g. intermediate commits might break 
tests fixed by subsequent commits, or intermediate commits might not 
build at all. Squashing before merging does eliminate those risks, but 
I recommend code review instead: commit history and style is subject to 
review just like code changes are. Sometimes I see a merge request with 
a messy commit history that just begs for two or more commits to be 
squashed together. (This is common for students new to open source. 
WebKit does not have this problem currently.) Sometimes I see the 
opposite, where too many changes are bundled into one big commit. (This 
is more common in WebKit, since we normally try to have one patch per 
bug, and splitting changes into multiple bugs is inconvenient.) But 
usually the developer submitting a MR has found a good balance between 
the two extremes. Ultimately, what's most important is landing a clean 
commit history with reviewable commits. Again, the scope of commits is 
subject to review just like code changes are.


I agree that we don't want to merge WIP commits of any form, and 
reviewers should complain if these appear in a MR. E.g. if making an 
API change that requires updates in 200 files, we surely want them all 
updated in one commit, because we don't want broken intermediate 
commits on master that would break bisections. So sometimes huge 
commits are unavoidable. Similarly, if an intermediate commit is known 
to break a test, that's not good either because it could mess up 
bisects. (I don't think we necessarily need to run tests on every 
intermediate commit -- that might be too much for our infrastructure -- 
but we could if desired.) What we don't want is to wind up merging 
low-quality stream-of-consciousness style commits that looks like they 
were committed in the order the code was written. I think that's what 
you're trying to avoid by suggesting squashes. Instead, I suggest 
developers should aggressively use 'git add -p' and 'git rebase -i' to 
selectively commit, rewrite, and reorder history to look good before 
opening the MR. This isn't optional for most open source projects: if 
you propose an MR with ugly commit history, it won't be merged until 
fixed. For a typical MR of moderate complexity, I'll use 'git rebase 
-i' at least a couple times before the history is clean enough for a 
MR, and to make fixup commits disappear into the most-appropriate 
commit in the sequence.


Regarding commit messages, I don't understand why there's any 
expectation that they need to be checked into files in order to be 
detailed and complete. If a commit message doesn't adequately describe 
what it's fixing, then the reviewer should open a

Re: [webkit-dev] WebKit Transition to Git

2020-10-13 Thread Maciej Stachowiak



> On Oct 3, 2020, at 9:42 AM, Tetsuharu OHZEKI  
> 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.ohz...@gmail.com 
> 
> On Sat, Oct 3, 2020 at 7:17 PM Ryosuke Niwa  > wrote:
>> 
>> On Sat, Oct 3, 2020 at 2:25 AM Adrien Destugues
>> mailto:pulkoma...@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 mes

Re: [webkit-dev] WebKit Transition to Git

2020-10-13 Thread Maciej Stachowiak



> On Oct 2, 2020, at 10:59 AM, Michael Catanzaro  wrote:
> 
> On Fri, Oct 2, 2020 at 6:36 pm, Philippe Normand  wrote:
>> Would you also consider preventing merge commits in order to keep a
>> clean mainline branch?
> 
> Big +1 to blocking merge commits. Merge commits in a huge project like WebKit 
> would make commit archaeology very frustrating. (I assume this is implied by 
> the monotonic commit identifiers proposal, but it doesn't exactly say that.)

I’m assuming your objection is to regular merges, but how do you feel about 
squash merges? Or do you think all PRs should be landed by rebasing?

My own preference would be to require squash merge, because it keeps the 
history simple for the main branch, but does not risk putting intermediate 
revisions which may work or even build on the main branch.

 - Maciej

> 
> I'm sure transition to git and GitHub should go well. I would have selected 
> GitLab myself -- it's nicer and also overwhelmingly popular -- but whatever. 
> (Does GitHub have merge request approvals? Replicating our reviewer/owner 
> permissions with GitLab merge request approvals would be easy.)
> 
> One downside is that using github.com might actually make it *too* easy to 
> spam us with low-quality issue reports and merge requests. We've historically 
> been pretty bad at maintaining a clean issue tracker -- the quantity of 
> untriaged issues on Bugzilla is very high -- and GitHub will make this worse. 
> That's not an issue with the GitHub platform, though. Just something to stay 
> on top of.
> 
> 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] WebKit Transition to Git

2020-10-07 Thread Michael[tm] Smith

Adrian Perez de Castro , 2020-10-08 00:49 +0300:
> ...
> Also with Git{Hub,Lab} it's an annoyance that one has to go through the web
> interface to create a “pull request”.

But it’s not hard to create tooling that completely eliminates the need to
ever use the GitHub web UI to create pull requests, right? And specifically,
I think we already have some good examples of automating integration of pull-
request creation into existing workflows for browser-project engineers.

https://github.com/web-platform-tests/wpt/pulls for example, is one well-known
place in the overall browser-project world where PR creation is hooked closely
into existing browser-project workflows and automation/CI; specifically:

* https://github.com/web-platform-tests/wpt/pulls/chromium-wpt-export-bot
* https://github.com/web-platform-tests/wpt/pulls/moz-wptsync-bot

Those are the pull-request histories for all tests that have been automatically
upstreamed from the Chromium and Firefox projects to WPT through PRs.

And yeah those are PRs for tests, not browser source-code patches, but it’s easy
to imagine how patch handling too — or any part of a browser-project workflow
that makes use of pull requests — could be integrated into a GitHub repo without
the need for engineers to use the GitHub web UI to do PR creation directly.

  –Mike

P.S. As far as whether it’s ever necessary to use the GitHub web UI for PR
creation even in the general case: I create a lot of GitHub PRs for a
variety of projects, and I pretty much never use the GitHub Web UI to
create them. Instead I just use https://hub.github.com/ from the command
line. And there’s https://cli.github.com/ and other CLI tools for doing it.

-- 
Michael[tm] Smith https://people.w3.org/mike


signature.asc
Description: PGP signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebKit Transition to Git

On Tue, 06 Oct 2020 11:15:05 -0700 Jonathan Bedard  wrote:

> Seems like most large projects are using some sort of bot or action to solve
> this problem: https://github.com/isaacs/github/issues/581
> .

Wow, “there's a bot for that” seems to be the “there's an app for that” of
the GitHub ecosystem 😵

> > On Oct 6, 2020, at 11:07 AM, Michael Catanzaro  wrote:
> > 
> > On Wed, Oct 7, 2020 at 2:22 am, Tetsuharu OHZEKI 
> >  wrote:
> >> If we move to GitHub Issue, compared to bugzilla, that does not have
> >> "component watching".
> >> (I don't know the case of GitLab's Issue)
> >> We only can watch all issues or not for the repository.
> > 
> > Oh dear. I had assumed that you could easily subscribe to particular labels 
> > (as you can in GitLab) but, poking around in GitHub's UI, I indeed don't 
> > see any way to do that. :/
> > 
> > That's no good. E.g. multimedia developers expect to be able to subscribe 
> > only to multimedia bugs, WebKitGTK developers will want to watch only GTK 
> > bugs, etc.
> > 
> > Michael


pgpGPdP1onlz3.pgp
Description: PGP signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebKit Transition to Git

Hi there,

There is one more annoyance I am adding below Konstantin's list…

On Tue, 06 Oct 2020 03:13:32 +0300 Konstantin Tokarev 
wrote:
 
> 05.10.2020, 23:41, "Yusuke Suzuki" :
> > I think security component is special in terms of how to handle it already
> > (e.g. not posting a test with the patch etc.) To me, handling non-security
> > issues in GitHub and security issues in Bugzilla is OK.  By nature,
> > security issues are not open. Since one of our motivation of moving to
> > GitHub is openness for feedback collection, security issue in Bugzilla
> > does not matter for this motivation.  Ideally, handling both in GitHub is
> > better. But to me, rather than continuing using Bugzilla, using GitHub for
> > non security issues sounds improvement.
> 
> To me it sounds as a huge step backwards. Asides from situation with
> security issues, it has other significant drawbacks in domain of issue
> triaging and management:
> 
> 1. Sub-par support for linking issues to each other
> 
> 
> Traditional bug tracking systems (like Bugzilla or JIRA) have support of
> "related" or "linked" issues. Most important relations are
> 
> * A depends on B (B blocks A) - blockers and umbrella issues
>
> * B is duplicate of A
>
> * A and B are related in other unspecified way
> 
> All GitHub can offer here now is mentions (and, to some extent, milestones
> for case of "umbrella issues" [1]). Mention is created every time someone
> uses "#" (e.g. "#123") in the text of issue or in the comment, where
> number is a sequential number of issue or pull request [2]. When comment is
> published in issue A which mentions issue B, there is a pseudo-comment added
> to B, and subscribers of B receive email notification.
> 
> At first glance this naive approach seems to work, but
> 
> * There is no easily visible list of relations: if you are not closely
>   following all activity on A, to find all issues related to it you have
>   to browse through all its (pseudo-)comments, which in some cases might
>   be long.
>
> * There is no *stateful* list of relations: if A was believed to have
>   common source with B, but then it was discovered they are not related,
>   you cannot delete relationship between A and B because there is no
>   relationship, just a set of comments.
>
> * "#" is not a safe reference format. Sometimes users' comments
>   may have other data in "#" format with a different
>   meaning than references to GitHub issues. For example, may the force be
>   with you if someone pastes gdb or lldb backtrace into comment without
>   escaping it into markdown raw text block (```). Also, GitHub parses
>   mentions in git commit messages, so care must be taken to avoid any
>   occurrences of "#" with a meaning different from reference to
>   issue number.
> 
> ---
> 
> [1] Milestones are not issues themselves, but they establish true two-way
> stateful relation between issues and their "parent" milestone.  [2] For some
> reason they have shared numbering, which means sometimes you may not know
> what kind of reference is in front of you until you look up its URL or
> navigate
> 
> 
> 2. Sub-par issue categorization and search
> --
> 
> In traditional bug tracking systems every issue has properties like status,
> resolution, component, severity/issue type, priority. When new bug is
> created, user chooses values of these properties, which can be later
> adjusted by the person doing triaging. They also are used in user-friendly
> search dialog like [1].
> 
> All GitHub can offer here are custom labels. While in theory they are
> sufficient to replace pre-defined issue properties, they require disciplined
> use to be efficient, for example issue must not have mutually exclusive
> labels. To avoid chaos, GitHub allows setting issue labels only to
> contributors. However, this puts more work on triagers, and they cannot
> triage only certain kinds of issues which match their interests by going
> through results of search query.
> 
> [1] https://bugs.webkit.org/query.cgi
> 
> 
> 3. Sub-par attachments
> --
> 
> Traditional bug trackers allow attaching files to issue. GitHub goes further
> and allows to attach files to every comment. Enjoy the progress -  now you
> can look for attached test cases and proposed patches through all comment
> feed, instead of having them in one place at the top.
> 
> Also, on test cases. You probably like this feature of Bugzilla when you can
> attach self-contained HTML file to the issue and then simply open it by URL
> in any browser including your build of WebKit to try it out. Forget this -
> GitHub simply forbids HTML or JS attachments (without wrapping them in
> archive):
> 
> "We don’t support that file type. with a GIF, JPEG, JPG, PNG, DOCX, GZ,
> LOG, PDF, PPTX, TXT, XLSX or ZIP."
> 
> And yes, take care not to use tar.xz or tar.bz2 or any other unapprov

Re: [webkit-dev] WebKit Transition to Git

On Sun, 04 Oct 2020 02:00:02 +0300 Konstantin Tokarev  wrote:

> 03.10.2020, 21:49, "Adrien Destugues" :
> > Yes that's why I didn't elaborate much. Whichever tool you pick, there
> > will always be people unhappy about it.
> 
> Right. For example, I have negative bias against GitLab, because this
> company have bought its open source competitor (Gitorious, which was
> quite popular service those days) just to shut it down.

I personally have negative bias against both GitHub *and* GitLab.

In the case of GitHub, apart of the reasons already mentioned by others,
yesterday I started getting a new popup asking consent to being tracked
even more.

In the case of GitLab, it's a bloated kitchen sink which does not inspire
trust in me when its official documentation has pearls [1] like this:

  “GitLab has memory leaks. These memory leaks manifest themselves in
   long-running processes, such as Unicorn workers. (The Unicorn master
   process is not known to leak memory, probably because it does not
   handle user requests.)

   To make these memory leaks manageable, GitLab comes with the
   unicorn-worker-killer gem. This gem monkey-patches the Unicorn workers to
   do a memory self-check after every 16 requests. If the memory of the
   Unicorn worker exceeds a pre-set limit then the worker process exits. The
   Unicorn master then automatically replaces the worker process.”

Also GitLab has contracts with ICE (like GitHub), and not only explicitly
decided to continue them, but also attempted to silence their employees
on political matters:

  https://www.theregister.com/2019/10/16/gitlab_employees_gagged/
  https://www.theregister.com/2019/10/17/gitlab_reverse_ferret/

Cheers,
—Adrián

---
[1] 
https://docs.gitlab.com/ee/administration/operations/unicorn.html#unicorn-worker-killer

pgp4KjNmxjOrc.pgp
Description: PGP signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebKit Transition to Git

Hello,

On Fri, 02 Oct 2020 13:48:56 -0700 Ken Russell  wrote:

> Excited about this migration! One comment inline.
> 
> On Fri, Oct 2, 2020 at 9:43 AM Jonathan Bedard  wrote:
> 
> > *Patches to Pull Requests*
> > In the coming months, more automation will start using the git version of
> > the repository instead of the Subversion version. Even immediately after we
> > switch, the patch review workflow will remain what it is now (EWS bots
> > mostly already use a git clone as their checkout). We do, however, intend
> > to switch from a system of patch review to a system of pull requests. This
> > process will be incremental and the patch review system will remain alive
> > as long as Bugzilla does.
> >
> > *GitHub Issues*
> > The last part of transitioning away from Subversion is to re-evaluate our
> > bug tracker. Bugzilla has served us well, but seems to be an impediment to
> > engaging with the open source community. We are interested in moving away
> > from Bugzilla and to GitHub Issues, allowing new developers to work with a
> > system they are likely already familiar with and to allow us closer
> > integration with repositories managed by W3C. Although GitHub Issues has
> > had performance problems in the past with projects that have many bugs,
> > these performance problems have been resolved to the satisfaction of a few
> > large projects hosted on GitHub that are of a comparable scale to WebKit.
> > The biggest blocker we are aware of is managing security bugs, since the
> > security advisory system used by GitHub is essentially the opposite of how
> > WebKit security bugs work. Moving to GitHub Issues, if it happens, will be
> > the last part of this transition, and we are interested in soliciting
> > feedback from our contributors on what the WebKit project’s integration
> > with GitHub Issues should look like.
> 
> Github's code review UI has a couple of feature gaps in my opinion. It's
> difficult to look at earlier versions of the pull request, in particular to
> verify that issues found during code review have been fixed. I remember it
> also being difficult to figure out whether all comments on earlier versions
> have been addressed.

There is also one annoyance which has been slowly grinding my gears: the
GitHub diff viewer (including in reviews) hides “big diffs” for some arbitrary
and unknown definition of “big”, forcing a reviewer to manually click in a
“Load diff” link, for each hidden diff, every time. In a few cases I ran into
“very big” diffs or diff hunks which could not even be displayed that way and
had to fetch the branch locally to review the changes. Truth be told: it has
been a good while since I haven't found any diff which could not be loaded
at all, so maybe that issue has been fixed by now.

Now, GitLab's diff viewer has its own set of similar-but-not-quite-same
annoyances. Instead of hiding “big” diff hunks, it hides “random” hunks:
typically big ones, but sometimes small ones as well. A positive point for
GitLab is that at least they can always be loaded using that pesky “Load
diff” link.

Why none of Git{Hub,Lab} would have an option “gimme all the diffs” user
option, or at least remember which reviews I visited before where the “Load
diff” links have been used is something inexplicable. Or they could just
show the diffs ¯\_(ツ)_/¯ 

> Gerrit code review has worked well in my experience. It explicitly manages
> different versions of the same patch set, and prominently shows whether all
> code review comments have been addressed.

One point for Gerrit: I never ran into a diff that it would not show.

Regards,
—Adrián

pgpstt94v6F6n.pgp
Description: PGP signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebKit Transition to Git

Hello,

On Sun, 04 Oct 2020 01:57:56 -0700 Yusuke Suzuki  wrote:

> GitHub is planning to have review revisions in 2020/Q4.
> So, when WebKit repository is migrated, we already have that in GitHub 
> builtin review tool :)
> 
> https://github.com/github/roadmap/issues/54 
> 

It's not clear from the description in that issue that using a “rebase and
force-push” strategy to update patch sets to be will be supported and that the
differences between versions will be available. Rebases are not mentioned at
all, so I would not count on this workflow working even with the upcoming
improvements in 2020/Q4.
 
> > On Oct 3, 2020, at 5:18 PM, Michael Catanzaro  wrote:
> > 
> > On Sat, Oct 3, 2020 at 9:52 pm, Konstantin Tokarev  
> > wrote:
> >> If I understand correctly, there is no way in GitHub UI to see a 
> >> difference between
> >> patches submitted at step 1 and step 3. It's still possible to see old 
> >> version of patches
> >> if you navigate to old commit hash, but that's not for long as they can be 
> >> garbage
> >> collected by GitHub because they don't belong to git branch anymore.
> > 
> > If we are really seriously concerned about this... it's not a problem with
> > GitLab. At least, I can still view diffs between different revisions of
> > merge requests from two years ago, and the changes seem to be stored
> > forever. At least I still can view diffs from years ago. E.g.:
> > 
> > https://gitlab.gnome.org/GNOME/epiphany/-/merge_requests/62/diffs?diff_id=27669&start_sha=0c020384b602c9e1f0e8ec9e491d1951e8feadf7
> >
> > Unfortunately it's sometimes less useful than I might have hoped, because
> > rebases bring in a bunch of totally unrelated changes, e.g.:
> >
> > https://gitlab.gnome.org/GNOME/epiphany/-/merge_requests/62/diffs?diff_id=28036&start_sha=043b5fc32f4f9263d393c9de83e1b33123c5

This is the kind of thing that Gerrit is *designed* to solve :]

Cheers,
—Adrián


pgpd479NRitRQ.pgp
Description: PGP signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebKit Transition to Git

Hi,

On Tue, 06 Oct 2020 03:42:11 +0300 Konstantin Tokarev  wrote:

> 05.10.2020, 13:26, "Frédéric Wang" :
> > One thing to take into account is that WebKit's repository is big and
> > public GitHub/GitLab prevent creating large repository by default. This
> > means it might not be possible for contributors to actually fork
> > WebKit's repository on their account and then create a pull request
> > (which is the standard way IIUC).
> 
> Every repository which is already present on GitHub can be forked by
> anyone without any restrictions.
> 
> There are restrictions for repositories which are not forks, e.g. if you
> create new repo on your account and try to push whole WebKit there,
> you'll get an error.
> 
> > Instead, we would probably end up
> > doing like web-platform-tests and give contributors the permission to
> > create branches to the WebKit account and make Pull Request to the
> > master branch. 
> 
> This is not needed and should be avoided - private development branches
> in main repo will confuse users and will be copied into all forks.

While I agree that having private development branches in the main repository
can be confusing for people not used to that, OTOH it is awkward to have to
“fork” [1] the repository to one's user area, then add an extra remote to
one's local clone of the upstream (main) repository, just to be able to submit
a “pull request” [2].

Also with Git{Hub,Lab} it's an annoyance that one has to go through the web
interface to create a “pull request”. Being able to do “webkit-patch upload”
saves a non negligible amount of developer time. It is even better with
Gerrit: do “git push” directly to a remote ref and a changeset will be
automatically created for review [3].

> >Probably, we should forbid people to commit to the master
> > branch directly (I think someone broke WPT's master branch that way last
> > year)...
> 
> I have to admit that ability to make small unreviewed commit for thing like 
> typo fix or change in personal watchlist is rather useful...

I suppose this could be solved by still making a “pull request” and tagging
it with some label to be merged automatically by some bot. It won't be as
fast as directly landing a patch, but on the upside all patches (including
unreviewed ones) would be landed in the same way.

As a side note, Gerrit can be configured to allow direct pushes that bypass
the review process.

Cheers,
—Adrián

---
[1] I put “fork” in between quotes because it's a perversion of the term: what
it does is create a clone of the repository in Git{Hub,Lab}'s own servers
which happens to be accessible remotely with one's credentials.
[2] GitHub's usage of the “pull request” term is yet another perversion: Git
has had a pull requests (produced by a command) long before GitHub even
existed, see https://git-scm.com/docs/git-request-pull
[3] 
https://gerrit-documentation.storage.googleapis.com/Documentation/3.2.3/intro-user.html#upload-change

pgpEp7XcmnUrw.pgp
Description: PGP signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebKit Transition to Git

Hello,

On Wed, 07 Oct 2020 04:17:45 +0300 Konstantin Tokarev  wrote:
 
> 06.10.2020, 18:03, "Michael Catanzaro" :
> > On Tue, Oct 6, 2020 at 3:13 am, Konstantin Tokarev 
> > wrote:
> >>  1. Sub-par support for linking issues to each other
> >>  
> >>
> >>  Traditional bug tracking systems (like Bugzilla or JIRA) have support
> >>  of "related" or "linked" issues. Most important relations are
> >>
> >>  * A depends on B (B blocks A) - blockers and umbrella issues
> >>  * B is duplicate of A
> >>  * A and B are related in other unspecified way
> >
> > GitHub does have duplicates btw, but the syntax is completely different
> > from GitLab so I can never remember how it's done. It's not exposed in
> > the UI. In GitLab it's easy: /duplicate #245 to mark a bug as duplicate
> > of issue #245. (You can have cross-repo duplicates too, and I think
> > even cross-*project* duplicates. This is very important for GNOME, but
> > not for WebKit since WebKit is just one huge repo.)
> >
> > I found instructions for GitHub here:
> > https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/about-duplicate-issues-and-pull-requests#marking-duplicates
> >
> >>  * There is no easily visible list of relations: if you are not
> >>  closely following all activity on A, to find all issues related to it
> >>  you have to browse through all its (pseudo-)comments, which in some
> >>  cases might be long.
> >>  * There is no *stateful* list of relations: if A was believed to have
> >>  common source with B, but then it was discovered they are not
> >>  related, you cannot delete relationship between A and B because there
> >>  is no relationship, just a set of comments.
> >
> > In my experience, this isn't really a *huge* problem. Umbrella issues
> > work well enough to track relationship between bugs, as do milestones,
> > and you can use whichever you prefer. I admit it is not nearly as nice
> > as Depends/Blocks from Bugzilla, though. But I think it should be good
> > enough for us.
> >
> >>  * "#" is not a safe reference format. Sometimes users'
> >>  comments may have other data in "#" format with a different
> >>  meaning than references to GitHub issues. For example, may the force
> >>  be with you if someone pastes gdb or lldb backtrace into comment
> >>  without escaping it into markdown raw text block (```). Also, GitHub
> >>  parses mentions in git commit messages, so care must be taken to
> >>  avoid any occurrences of "#" with a meaning different from
> >>  reference to issue number.
> >
> > Yeah this is unfortunate and also guaranteed to happen. The first
> > several dozen issues are going to be spammed with references to other
> > bugs all over the place.
> >
> >>  [2] For some reason they have shared numbering, which means sometimes
> >>  you may not know what kind of reference is in front of you until you
> >>  look up its URL or navigate
> >
> > This is silly too (and not a problem on GitLab, which uses # for issues
> > and ! for merge requests). But although I agree it is a defect, is it
> > really a huge problem? Probably not.
> >
> >>  Also, on test cases. You probably like this feature of Bugzilla when
> >>  you can attach self-contained HTML file to the issue and then simply
> >>  open it by URL in any browser including your build of WebKit to try
> >>  it out. Forget this - GitHub simply forbids HTML or JS attachments
> >>  (without wrapping them in archive):
> >>
> >>  "We don’t support that file type. with a GIF, JPEG, JPG, PNG,
> >>  DOCX, GZ, LOG, PDF, PPTX, TXT, XLSX or ZIP."
> >>
> >>  And yes, take care not to use tar.xz or tar.bz2 or any other
> >>  unapproved archive type.
> >
> > OK, now you've convinced me... this is bad. :P I forgot about this
> > problem because GitLab does not have any such restrictions on
> > attachments. It's very frustrating to have to say "I added a .txt file
> > extension so I could upload this to GitHub. Please remove the file
> > extension after you have downloaded the file before you use it." I
> > would say this is strongest argument I've seen against GitHub. Silly
> > problem to have tbh.
> >
> > I'll just add that GitLab has become *really* popular for Linux-related
> > projects. I have to regularly work with GNOME GitLab, freedesktop
> > GitLab, and gitlab.com. Fedora and CentOS are both switching to GitLab
> > too, so soon that will be five different public GitLab instances that I
> > have to switch between regularly, and that's limited to only the public
> > instances. Then there are also many major communities I don't work with
> > that are also using GitLab (KDE, Debian, Purism).
> 
> FWIW, KDE is using Bugzilla for bug reporting, and Debian uses debbugs.

Not that debbugs would be particularly user-friendly… ;-)

> > It's reached the
> > point where unless you only work on Linux kernel itself, you probably
> > spend a lot of time on one GitLab or another. It should be consid

Re: [webkit-dev] WebKit Transition to Git

On Wed, 07 Oct 2020 08:58:58 -0500 Michael Catanzaro  
wrote:
> On Wed, Oct 7, 2020 at 4:17 am, Konstantin Tokarev  
> wrote:
> > BTW, could you estimate how many of users which have provided
> > meaningful bug reports were directed to bugs.webkit.org but never 
> > made it?
> 
> A lot. I'd say maybe about half make it upstream. That's OK. We don't 
> have time to fix everything, after all, and it doesn't make sense to 
> focus on bug reports where we cannot easily interact with the reporter.

I think there is some bias here to say that “a lot have not gone into
Bugzilla” because people who may complain about Bugzilla are of course more
vocal than those who went ahead, made an account (or already had one), and
filed a bug.

Also, the “a lot” above is definitely overestimated, because you do not seem
to be taking into account that among the people who cannot be bothered to file
a bug report in Bugzilla, many would not file it in GitHub either (nor GitLab,
nor anything else).

Good software gets its job done and is mostly invisible and boring: you use
it, but don't think twice about it, nor there is any need to write about it.
Bugzilla is among the kind of software that is good at what it does without
getting in the way, so of course it does not have hype. It's only when one
starts to seriously think about the allegedly “shiny” alternatives that all
the small annoyances start becoming evident (the list shared by Konstantin is
a perfect example).

Cheers,
—Adrián

pgpATre9rjMqT.pgp
Description: PGP signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebKit Transition to Git

2020-10-07 Thread Michael Catanzaro

On Wed, Oct 7, 2020 at 4:17 am, Konstantin Tokarev  
wrote:

BTW, could you estimate how many of users which have provided
meaningful bug reports were directed to bugs.webkit.org but never 
made it?


A lot. I'd say maybe about half make it upstream. That's OK. We don't 
have time to fix everything, after all, and it doesn't make sense to 
focus on bug reports where we cannot easily interact with the reporter.



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

Re: [webkit-dev] WebKit Transition to Git

2020-10-07 Thread Sam Sneddon

> On 6 Oct 2020, at 18:32, Yusuke Suzuki  wrote:
> 
> I don’t think this works. People in twitter etc. do not file a bug even if it 
> is requested by us not because Bugzilla has technical difficulties, just 
> because “This is not GitHub”.
> So, moving to GitLab / making Bugzilla easy do not solve the problem.
> I think GitHub is selected not because GitHub has XXX feature. This is 
> because GitHub has significant popularity over the other services, and a lot 
> of people are familiar to how GitHub works.

It might be interesting to ask people from Mozilla (I know some of you lurk 
here!) about what their experience has been after they added federated GitHub 
login to their branch of Bugzilla.

(To be clear, it very much is significantly branched, and I don’t think us 
moving to that branch is practical or therefore likely. But it would show if 
federated login would solve the issue, which at least makes GitLab more viable.)

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

Re: [webkit-dev] WebKit Transition to Git



06.10.2020, 18:03, "Michael Catanzaro" :
> On Tue, Oct 6, 2020 at 3:13 am, Konstantin Tokarev 
> wrote:
>>  1. Sub-par support for linking issues to each other
>>  
>>
>>  Traditional bug tracking systems (like Bugzilla or JIRA) have support
>>  of "related" or "linked" issues. Most important relations are
>>
>>  * A depends on B (B blocks A) - blockers and umbrella issues
>>  * B is duplicate of A
>>  * A and B are related in other unspecified way
>
> GitHub does have duplicates btw, but the syntax is completely different
> from GitLab so I can never remember how it's done. It's not exposed in
> the UI. In GitLab it's easy: /duplicate #245 to mark a bug as duplicate
> of issue #245. (You can have cross-repo duplicates too, and I think
> even cross-*project* duplicates. This is very important for GNOME, but
> not for WebKit since WebKit is just one huge repo.)
>
> I found instructions for GitHub here:
> https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/about-duplicate-issues-and-pull-requests#marking-duplicates
>
>>  * There is no easily visible list of relations: if you are not
>>  closely following all activity on A, to find all issues related to it
>>  you have to browse through all its (pseudo-)comments, which in some
>>  cases might be long.
>>  * There is no *stateful* list of relations: if A was believed to have
>>  common source with B, but then it was discovered they are not
>>  related, you cannot delete relationship between A and B because there
>>  is no relationship, just a set of comments.
>
> In my experience, this isn't really a *huge* problem. Umbrella issues
> work well enough to track relationship between bugs, as do milestones,
> and you can use whichever you prefer. I admit it is not nearly as nice
> as Depends/Blocks from Bugzilla, though. But I think it should be good
> enough for us.
>
>>  * "#" is not a safe reference format. Sometimes users'
>>  comments may have other data in "#" format with a different
>>  meaning than references to GitHub issues. For example, may the force
>>  be with you if someone pastes gdb or lldb backtrace into comment
>>  without escaping it into markdown raw text block (```). Also, GitHub
>>  parses mentions in git commit messages, so care must be taken to
>>  avoid any occurrences of "#" with a meaning different from
>>  reference to issue number.
>
> Yeah this is unfortunate and also guaranteed to happen. The first
> several dozen issues are going to be spammed with references to other
> bugs all over the place.
>
>>  [2] For some reason they have shared numbering, which means sometimes
>>  you may not know what kind of reference is in front of you until you
>>  look up its URL or navigate
>
> This is silly too (and not a problem on GitLab, which uses # for issues
> and ! for merge requests). But although I agree it is a defect, is it
> really a huge problem? Probably not.
>
>>  Also, on test cases. You probably like this feature of Bugzilla when
>>  you can attach self-contained HTML file to the issue and then simply
>>  open it by URL in any browser including your build of WebKit to try
>>  it out. Forget this - GitHub simply forbids HTML or JS attachments
>>  (without wrapping them in archive):
>>
>>  "We don’t support that file type. with a GIF, JPEG, JPG, PNG,
>>  DOCX, GZ, LOG, PDF, PPTX, TXT, XLSX or ZIP."
>>
>>  And yes, take care not to use tar.xz or tar.bz2 or any other
>>  unapproved archive type.
>
> OK, now you've convinced me... this is bad. :P I forgot about this
> problem because GitLab does not have any such restrictions on
> attachments. It's very frustrating to have to say "I added a .txt file
> extension so I could upload this to GitHub. Please remove the file
> extension after you have downloaded the file before you use it." I
> would say this is strongest argument I've seen against GitHub. Silly
> problem to have tbh.
>
> I'll just add that GitLab has become *really* popular for Linux-related
> projects. I have to regularly work with GNOME GitLab, freedesktop
> GitLab, and gitlab.com. Fedora and CentOS are both switching to GitLab
> too, so soon that will be five different public GitLab instances that I
> have to switch between regularly, and that's limited to only the public
> instances. Then there are also many major communities I don't work with
> that are also using GitLab (KDE, Debian, Purism).

FWIW, KDE is using Bugzilla for bug reporting, and Debian uses debbugs.

> It's reached the
> point where unless you only work on Linux kernel itself, you probably
> spend a lot of time on one GitLab or another. It should be considered
> alongside GitHub as an option, especially if we're concerned about
> GitHub-specific problems.

This is certainly non-representative, but among the projects /me personally 
have interacted recently almost all use GitHub Issues, Bugzilla, or JIRA for
bug reports.

>
>>  And when issues are triaged, they

Re: [webkit-dev] WebKit Transition to Git



06.10.2020, 20:32, "Yusuke Suzuki" :
>>  On Oct 5, 2020, at 5:13 PM, Konstantin Tokarev  wrote:
>>
>>  05.10.2020, 23:41, "Yusuke Suzuki" :
>>>  I think security component is special in terms of how to handle it already 
>>> (e.g. not posting a test with the patch etc.)
>>>  To me, handling non-security issues in GitHub and security issues in 
>>> Bugzilla is OK.
>>>  By nature, security issues are not open. Since one of our motivation of 
>>> moving to GitHub is openness for feedback collection, security issue in 
>>> Bugzilla does not matter for this motivation.
>>>  Ideally, handling both in GitHub is better. But to me, rather than 
>>> continuing using Bugzilla, using GitHub for non security issues sounds 
>>> improvement.
>>
>>  To me it sounds as a huge step backwards. Asides from situation with 
>> security issues, it has other significant drawbacks in domain of issue 
>> triaging and management:
>>
>>  1. Sub-par support for linking issues to each other
>>  
>>
>>  Traditional bug tracking systems (like Bugzilla or JIRA) have support of 
>> "related" or "linked" issues. Most important relations are
>>
>>  * A depends on B (B blocks A) - blockers and umbrella issues
>>  * B is duplicate of A
>>  * A and B are related in other unspecified way
>>
>>  All GitHub can offer here now is mentions (and, to some extent, milestones 
>> for case of "umbrella issues" [1]). Mention is created every time someone 
>> uses "#" (e.g. "#123") in the text of issue or in the comment, where 
>> number is a sequential number of issue or pull request [2]. When comment is 
>> published in issue A which mentions issue B, there is a pseudo-comment added 
>> to B, and subscribers of B receive email notification.
>>
>>  At first glance this naive approach seems to work, but
>>
>>  * There is no easily visible list of relations: if you are not closely 
>> following all activity on A, to find all issues related to it you have to 
>> browse through all its (pseudo-)comments, which in some cases might be long.
>>  * There is no *stateful* list of relations: if A was believed to have 
>> common source with B, but then it was discovered they are not related, you 
>> cannot delete relationship between A and B because there is no relationship, 
>> just a set of comments.
>>  * "#" is not a safe reference format. Sometimes users' comments may 
>> have other data in "#" format with a different meaning than 
>> references to GitHub issues. For example, may the force be with you if 
>> someone pastes gdb or lldb backtrace into comment without escaping it into 
>> markdown raw text block (```). Also, GitHub parses mentions in git commit 
>> messages, so care must be taken to avoid any occurrences of "#" with 
>> a meaning different from reference to issue number.
>>
>>  ---
>>
>>  [1] Milestones are not issues themselves, but they establish true two-way 
>> stateful relation between issues and their "parent" milestone.
>>  [2] For some reason they have shared numbering, which means sometimes you 
>> may not know what kind of reference is in front of you until you look up its 
>> URL or navigate
>>
>>  2. Sub-par issue categorization and search
>>  --
>>
>>  In traditional bug tracking systems every issue has properties like status, 
>> resolution, component, severity/issue type, priority. When new bug is 
>> created, user chooses values of these properties, which can be later 
>> adjusted by the person doing triaging. They also are used in user-friendly 
>> search dialog like [1].
>>
>>  All GitHub can offer here are custom labels. While in theory they are 
>> sufficient to replace pre-defined issue properties, they require disciplined 
>> use to be efficient, for example issue must not have mutually exclusive 
>> labels. To avoid chaos, GitHub allows setting issue labels only to 
>> contributors. However, this puts more work on triagers, and they cannot 
>> triage only certain kinds of issues which match their interests by going 
>> through results of search query.
>>
>>  [1] https://bugs.webkit.org/query.cgi
>>
>>  3. Sub-par attachments
>>  --
>>
>>  Traditional bug trackers allow attaching files to issue. GitHub goes 
>> further and allows to attach files to every comment. Enjoy the progress - 
>> now you can look for attached test cases and proposed patches through all 
>> comment feed, instead of having them in one place at the top.
>>
>>  Also, on test cases. You probably like this feature of Bugzilla when you 
>> can attach self-contained HTML file to the issue and then simply open it by 
>> URL in any browser including your build of WebKit to try it out. Forget this 
>> - GitHub simply forbids HTML or JS attachments (without wrapping them in 
>> archive):
>>
>> "We don’t support that file type. with a GIF, JPEG, JPG, PNG, DOCX, GZ, 
>> LOG, PDF, PPTX, TXT, XLSX or ZIP."
>>
>>  And yes, t

Re: [webkit-dev] WebKit Transition to Git

2020-10-06 Thread Jonathan Bedard

Seems like most large projects are using some sort of bot or action to solve 
this problem: https://github.com/isaacs/github/issues/581 
.

Jonathan

> On Oct 6, 2020, at 11:07 AM, Michael Catanzaro  wrote:
> 
> On Wed, Oct 7, 2020 at 2:22 am, Tetsuharu OHZEKI  
> wrote:
>> If we move to GitHub Issue, compared to bugzilla, that does not have
>> "component watching".
>> (I don't know the case of GitLab's Issue)
>> We only can watch all issues or not for the repository.
> 
> Oh dear. I had assumed that you could easily subscribe to particular labels 
> (as you can in GitLab) but, poking around in GitHub's UI, I indeed don't see 
> any way to do that. :/
> 
> That's no good. E.g. multimedia developers expect to be able to subscribe 
> only to multimedia bugs, WebKitGTK developers will want to watch only GTK 
> bugs, etc.
> 
> Michael
> 
> 

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

Re: [webkit-dev] WebKit Transition to Git

2020-10-06 Thread Michael Catanzaro

On Wed, Oct 7, 2020 at 2:22 am, Tetsuharu OHZEKI 
 wrote:

If we move to GitHub Issue, compared to bugzilla, that does not have
"component watching".
(I don't know the case of GitLab's Issue)
We only can watch all issues or not for the repository.


Oh dear. I had assumed that you could easily subscribe to particular 
labels (as you can in GitLab) but, poking around in GitHub's UI, I 
indeed don't see any way to do that. :/


That's no good. E.g. multimedia developers expect to be able to 
subscribe only to multimedia bugs, WebKitGTK developers will want to 
watch only GTK bugs, etc.


Michael


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

Re: [webkit-dev] WebKit Transition to Git

2020-10-06 Thread Yusuke Suzuki



> On Oct 5, 2020, at 5:13 PM, Konstantin Tokarev  wrote:
> 
> 
> 
> 05.10.2020, 23:41, "Yusuke Suzuki" :
>> I think security component is special in terms of how to handle it already 
>> (e.g. not posting a test with the patch etc.)
>> To me, handling non-security issues in GitHub and security issues in 
>> Bugzilla is OK.
>> By nature, security issues are not open. Since one of our motivation of 
>> moving to GitHub is openness for feedback collection, security issue in 
>> Bugzilla does not matter for this motivation.
>> Ideally, handling both in GitHub is better. But to me, rather than 
>> continuing using Bugzilla, using GitHub for non security issues sounds 
>> improvement.
> 
> To me it sounds as a huge step backwards. Asides from situation with security 
> issues, it has other significant drawbacks in domain of issue triaging and 
> management:
> 
> 1. Sub-par support for linking issues to each other
> 
> 
> Traditional bug tracking systems (like Bugzilla or JIRA) have support of 
> "related" or "linked" issues. Most important relations are
> 
> * A depends on B (B blocks A) - blockers and umbrella issues
> * B is duplicate of A
> * A and B are related in other unspecified way
> 
> All GitHub can offer here now is mentions (and, to some extent, milestones 
> for case of "umbrella issues" [1]). Mention is created every time someone 
> uses "#" (e.g. "#123") in the text of issue or in the comment, where 
> number is a sequential number of issue or pull request [2]. When comment is 
> published in issue A which mentions issue B, there is a pseudo-comment added 
> to B, and subscribers of B receive email notification.
> 
> At first glance this naive approach seems to work, but
> 
> * There is no easily visible list of relations: if you are not closely 
> following all activity on A, to find all issues related to it you have to 
> browse through all its (pseudo-)comments, which in some cases might be long. 
> * There is no *stateful* list of relations: if A was believed to have common 
> source with B, but then it was discovered they are not related, you cannot 
> delete relationship between A and B because there is no relationship, just a 
> set of comments.
> * "#" is not a safe reference format. Sometimes users' comments may 
> have other data in "#" format with a different meaning than 
> references to GitHub issues. For example, may the force be with you if 
> someone pastes gdb or lldb backtrace into comment without escaping it into 
> markdown raw text block (```). Also, GitHub parses mentions in git commit 
> messages, so care must be taken to avoid any occurrences of "#" with 
> a meaning different from reference to issue number.
> 
> ---
> 
> [1] Milestones are not issues themselves, but they establish true two-way 
> stateful relation between issues and their "parent" milestone.
> [2] For some reason they have shared numbering, which means sometimes you may 
> not know what kind of reference is in front of you until you look up its URL 
> or navigate
> 
> 
> 2. Sub-par issue categorization and search
> --
> 
> In traditional bug tracking systems every issue has properties like status, 
> resolution, component, severity/issue type, priority. When new bug is 
> created, user chooses values of these properties, which can be later adjusted 
> by the person doing triaging. They also are used in user-friendly search 
> dialog like [1].
> 
> All GitHub can offer here are custom labels. While in theory they are 
> sufficient to replace pre-defined issue properties, they require disciplined 
> use to be efficient, for example issue must not have mutually exclusive 
> labels. To avoid chaos, GitHub allows setting issue labels only to 
> contributors. However, this puts more work on triagers, and they cannot 
> triage only certain kinds of issues which match their interests by going 
> through results of search query.
> 
> [1] https://bugs.webkit.org/query.cgi
> 
> 
> 3. Sub-par attachments
> --
> 
> Traditional bug trackers allow attaching files to issue. GitHub goes further 
> and allows to attach files to every comment. Enjoy the progress -  now you 
> can look for attached test cases and proposed patches through all comment 
> feed, instead of having them in one place at the top.
> 
> Also, on test cases. You probably like this feature of Bugzilla when you can 
> attach self-contained HTML file to the issue and then simply open it by URL 
> in any browser including your build of WebKit to try it out. Forget this - 
> GitHub simply forbids HTML or JS attachments (without wrapping them in 
> archive):
> 
>"We don’t support that file type. with a GIF, JPEG, JPG, PNG, DOCX, GZ, 
> LOG, PDF, PPTX, TXT, XLSX or ZIP."
> 
> And yes, take care not to use tar.xz or tar.bz2 or any other unapproved 
> archive type.
> 
> But you can attach funny cat picture to your comm

Re: [webkit-dev] WebKit Transition to Git

2020-10-06 Thread Tetsuharu OHZEKI

I'm not sure about how many people in WebKit watches creating or updating bugs,
but I'd like to mention that because this might be a regression if we
move from bugzilla.

If we move to GitHub Issue, compared to bugzilla, that does not have
"component watching".
(I don't know the case of GitLab's Issue)
We only can watch all issues or not for the repository.

Today's GitHub Issue can move an issue to other repositories but I
don't see much that
some organization hosting a large scale project hosts multiple
repositories which corresponds to a _component_ of bugzilla
and triage an issue by moving on each repository.

Before GitHub implements to move an issue,
as my memory, for example Servo project created a bot to watch some
specific label and
send an email to a subscriber configured by a bot settings to do like
bugzilla's component watching.

I'm not sure about how many people in WebKit watches creating or updating bugs,
but I'd like to mention that because this might be a regression if we
move from bugzilla.


--
Tetsuharu OHZEKI
tetsuharu.ohz...@gmail.com

On Sat, Oct 3, 2020 at 1:43 AM Jonathan Bedard  wrote:
>
> Hello WebKit Contributors,
>
> This year, Apple would like to push WebKit’s source code management off of 
> Subversion and onto git. Our rationale for this is the rest of the industry 
> has settled on git as their source code management solution.  We’re also 
> interested in moving to a hosted Git solution (namely, GitHub) to make it 
> easier for new contributors to interact with the project. I would like to 
> outline our plan so far, and solicit feedback from our contributors about 
> some of the pieces we’re still discussing.
>
> Monotonic Commit Identifiers
> Of great interest to Apple’s engineers has been retaining some kind of 
> ordered tag we can use to refer to commits to make defending CI and bisection 
> easier. We’ve developed a scheme for this that assigns commits an ordered 
> identifier per-branch, outlined in 
> https://trac.webkit.org/wiki/commit-identifiers, designed to be used 
> alongside git hashes. These identifiers can be used in our current Subversion 
> repository, and we would like to start using them before the project has 
> transitions to git.
>
> Hosting the Repository
> We're most likely going to be hosting the repository in public GitHub. The 
> rationale for choosing GitHub over alternatives (namely, GitLab) is that 
> GitHub has a far more active community, and Apple would like WebKit to be 
> more connected to the open source community. For this reason, we prefer to 
> place the canonical WebKit repository on public GitHub so the project is more 
> accessible to new contributors, although some concerns have been raised about 
> GitHub’s terms and conditions, which contributors of WebKit would need to 
> agree to in addition to the terms and conditions of the WebKit project. We 
> are interested in what our current community of contributors thinks about 
> this, and what preferences contributors may have.
>
> Patches to Pull Requests
> In the coming months, more automation will start using the git version of the 
> repository instead of the Subversion version. Even immediately after we 
> switch, the patch review workflow will remain what it is now (EWS bots mostly 
> already use a git clone as their checkout). We do, however, intend to switch 
> from a system of patch review to a system of pull requests. This process will 
> be incremental and the patch review system will remain alive as long as 
> Bugzilla does.
>
> GitHub Issues
> The last part of transitioning away from Subversion is to re-evaluate our bug 
> tracker. Bugzilla has served us well, but seems to be an impediment to 
> engaging with the open source community. We are interested in moving away 
> from Bugzilla and to GitHub Issues, allowing new developers to work with a 
> system they are likely already familiar with and to allow us closer 
> integration with repositories managed by W3C. Although GitHub Issues has had 
> performance problems in the past with projects that have many bugs, these 
> performance problems have been resolved to the satisfaction of a few large 
> projects hosted on GitHub that are of a comparable scale to WebKit. The 
> biggest blocker we are aware of is managing security bugs, since the security 
> advisory system used by GitHub is essentially the opposite of how WebKit 
> security bugs work. Moving to GitHub Issues, if it happens, will be the last 
> part of this transition, and we are interested in soliciting feedback from 
> our contributors on what the WebKit project’s integration with GitHub Issues 
> should look like.
>
> Look forward to hearing from all of you,
>
> Jonathan Bedard
> WebKit Operations
> ___
> 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
htt

Re: [webkit-dev] WebKit Transition to Git

2020-10-06 Thread Michael Catanzaro

On Tue, Oct 6, 2020 at 3:13 am, Konstantin Tokarev
wrote:

1. Sub-par support for linking issues to each other

Traditional bug tracking systems (like Bugzilla or JIRA) have support
of "related" or "linked" issues. Most important relations are

* A depends on B (B blocks A) - blockers and umbrella issues
* B is duplicate of A
* A and B are related in other unspecified way

GitHub does have duplicates btw, but the syntax is completely different
from GitLab so I can never remember how it's done. It's not exposed in
the UI. In GitLab it's easy: /duplicate #245 to mark a bug as duplicate
of issue #245. (You can have cross-repo duplicates too, and I think
even cross-*project* duplicates. This is very important for GNOME, but
not for WebKit since WebKit is just one huge repo.)

I found instructions for GitHub here:
https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/about-duplicate-issues-and-pull-requests#marking-duplicates

* There is no easily visible list of relations: if you are not
closely following all activity on A, to find all issues related to it
you have to browse through all its (pseudo-)comments, which in some
cases might be long.
* There is no *stateful* list of relations: if A was believed to have
common source with B, but then it was discovered they are not
related, you cannot delete relationship between A and B because there
is no relationship, just a set of comments.

In my experience, this isn't really a *huge* problem. Umbrella issues
work well enough to track relationship between bugs, as do milestones,
and you can use whichever you prefer. I admit it is not nearly as nice
as Depends/Blocks from Bugzilla, though. But I think it should be good
enough for us.

* "#" is not a safe reference format. Sometimes users'
comments may have other data in "#" format with a different
meaning than references to GitHub issues. For example, may the force
be with you if someone pastes gdb or lldb backtrace into comment
without escaping it into markdown raw text block (```). Also, GitHub
parses mentions in git commit messages, so care must be taken to
avoid any occurrences of "#" with a meaning different from
reference to issue number.

Yeah this is unfortunate and also guaranteed to happen. The first
several dozen issues are going to be spammed with references to other
bugs all over the place.

[2] For some reason they have shared numbering, which means sometimes
you may not know what kind of reference is in front of you until you
look up its URL or navigate

This is silly too (and not a problem on GitLab, which uses # for issues
and ! for merge requests). But although I agree it is a defect, is it
really a huge problem? Probably not.

Also, on test cases. You probably like this feature of Bugzilla when
you can attach self-contained HTML file to the issue and then simply
open it by URL in any browser including your build of WebKit to try
it out. Forget this - GitHub simply forbids HTML or JS attachments
(without wrapping them in archive):

"We don’t support that file type. with a GIF, JPEG, JPG, PNG,
DOCX, GZ, LOG, PDF, PPTX, TXT, XLSX or ZIP."

And yes, take care not to use tar.xz or tar.bz2 or any other
unapproved archive type.

OK, now you've convinced me... this is bad. :P I forgot about this
problem because GitLab does not have any such restrictions on
attachments. It's very frustrating to have to say "I added a .txt file
extension so I could upload this to GitHub. Please remove the file
extension after you have downloaded the file before you use it." I
would say this is strongest argument I've seen against GitHub. Silly
problem to have tbh.

I'll just add that GitLab has become *really* popular for Linux-related
projects. I have to regularly work with GNOME GitLab, freedesktop
GitLab, and gitlab.com. Fedora and CentOS are both switching to GitLab
too, so soon that will be five different public GitLab instances that I
have to switch between regularly, and that's limited to only the public
instances. Then there are also many major communities I don't work with
that are also using GitLab (KDE, Debian, Purism). It's reached the
point where unless you only work on Linux kernel itself, you probably
spend a lot of time on one GitLab or another. It should be considered
alongside GitHub as an option, especially if we're concerned about
GitHub-specific problems.

And when issues are triaged, they can be resubmitted to WebKit
tracker by qualified person if they are really relevant.

In practice, I don't think it's a good idea because (a) moving bugs
upstream takes a lot of time, (b) it's quite often important to have
the original reporter CCed on the issue and able to comment. I receive
tons of WebKit bugs in the Epiphany bugtracker, and while a few years
ago I might have forwarded reports upstream myself, nowadays I only
provide instructio

Re: [webkit-dev] WebKit Transition to Git



06.10.2020, 12:40, "Konstantin Tokarev" :
> 06.10.2020, 11:42, "Ryosuke Niwa" :
>>  On Mon, Oct 5, 2020 at 5:13 PM Konstantin Tokarev  wrote:
>>>
>>>   1. Sub-par support for linking issues to each other
>>>   
>>>
>>>   Traditional bug tracking systems (like Bugzilla or JIRA) have support of 
>>> "related" or "linked" issues. Most important relations are
>>>
>>>   * A depends on B (B blocks A) - blockers and umbrella issues
>>>   * B is duplicate of A
>>>   * A and B are related in other unspecified way
>>>
>>>   All GitHub can offer here now is mentions (and, to some extent, 
>>> milestones for case of "umbrella issues" [1]). Mention is created every 
>>> time someone uses "#" (e.g. "#123") in the text of issue or in the 
>>> comment, where number is a sequential number of issue or pull request [2]. 
>>> When comment is published in issue A which mentions issue B, there is a 
>>> pseudo-comment added to B, and subscribers of B receive email notification.
>>>
>>>   At first glance this naive approach seems to work, but
>>>
>>>   * There is no easily visible list of relations: if you are not closely 
>>> following all activity on A, to find all issues related to it you have to 
>>> browse through all its (pseudo-)comments, which in some cases might be long.
>>>   * There is no *stateful* list of relations: if A was believed to have 
>>> common source with B, but then it was discovered they are not related, you 
>>> cannot delete relationship between A and B because there is no 
>>> relationship, just a set of comments.
>>>   * "#" is not a safe reference format. Sometimes users' comments 
>>> may have other data in "#" format with a different meaning than 
>>> references to GitHub issues. For example, may the force be with you if 
>>> someone pastes gdb or lldb backtrace into comment without escaping it into 
>>> markdown raw text block (```). Also, GitHub parses mentions in git commit 
>>> messages, so care must be taken to avoid any occurrences of "#" 
>>> with a meaning different from reference to issue number.
>>
>>  Yeah, this is a pretty significant functional regression to me. I use
>>  bug dependencies all the time (e.g.
>>  https://bugs.webkit.org/showdependencytree.cgi?id=148695&hide_resolved=1)
>>  and not having this capability will significantly hinder my ability to
>>  track & triage some bugs.
>
> As I've mentioned above, for this particular case you could create a 
> milestone "Implement v1 shadow DOM API" and add all subtasks to it. Then you 
> have a list of dependencies in one place, with ability to see only unresolved 
> once, and with a nice progress bar. But you may find this approach a bit 
> lame, because milestone itself is not a proper task, it has nothing else but 
> title, description, and due date. Also, it's probably wrong to call this 
> entity a milestone.

Also it doesn't allow to create dependency tree with depth > 1, as children of 
milestone cannot be milestones themselves.


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

Re: [webkit-dev] WebKit Transition to Git



06.10.2020, 11:42, "Ryosuke Niwa" :
> On Mon, Oct 5, 2020 at 5:13 PM Konstantin Tokarev  wrote:
>>  05.10.2020, 23:41, "Yusuke Suzuki" :
>>  > I think security component is special in terms of how to handle it 
>> already (e.g. not posting a test with the patch etc.)
>>  > To me, handling non-security issues in GitHub and security issues in 
>> Bugzilla is OK.
>>  > By nature, security issues are not open. Since one of our motivation of 
>> moving to GitHub is openness for feedback collection, security issue in 
>> Bugzilla does not matter for this motivation.
>>  > Ideally, handling both in GitHub is better. But to me, rather than 
>> continuing using Bugzilla, using GitHub for non security issues sounds 
>> improvement.
>>
>>  To me it sounds as a huge step backwards. Asides from situation with 
>> security issues, it has other significant drawbacks in domain of issue 
>> triaging and management:
>>
>>  1. Sub-par support for linking issues to each other
>>  
>>
>>  Traditional bug tracking systems (like Bugzilla or JIRA) have support of 
>> "related" or "linked" issues. Most important relations are
>>
>>  * A depends on B (B blocks A) - blockers and umbrella issues
>>  * B is duplicate of A
>>  * A and B are related in other unspecified way
>>
>>  All GitHub can offer here now is mentions (and, to some extent, milestones 
>> for case of "umbrella issues" [1]). Mention is created every time someone 
>> uses "#" (e.g. "#123") in the text of issue or in the comment, where 
>> number is a sequential number of issue or pull request [2]. When comment is 
>> published in issue A which mentions issue B, there is a pseudo-comment added 
>> to B, and subscribers of B receive email notification.
>>
>>  At first glance this naive approach seems to work, but
>>
>>  * There is no easily visible list of relations: if you are not closely 
>> following all activity on A, to find all issues related to it you have to 
>> browse through all its (pseudo-)comments, which in some cases might be long.
>>  * There is no *stateful* list of relations: if A was believed to have 
>> common source with B, but then it was discovered they are not related, you 
>> cannot delete relationship between A and B because there is no relationship, 
>> just a set of comments.
>>  * "#" is not a safe reference format. Sometimes users' comments may 
>> have other data in "#" format with a different meaning than 
>> references to GitHub issues. For example, may the force be with you if 
>> someone pastes gdb or lldb backtrace into comment without escaping it into 
>> markdown raw text block (```). Also, GitHub parses mentions in git commit 
>> messages, so care must be taken to avoid any occurrences of "#" with 
>> a meaning different from reference to issue number.
>
> Yeah, this is a pretty significant functional regression to me. I use
> bug dependencies all the time (e.g.
> https://bugs.webkit.org/showdependencytree.cgi?id=148695&hide_resolved=1)
> and not having this capability will significantly hinder my ability to
> track & triage some bugs.

As I've mentioned above, for this particular case you could create a milestone 
"Implement v1 shadow DOM API" and add all subtasks to it. Then you have a list 
of dependencies in one place, with ability to see only unresolved once, and 
with a nice progress bar. But you may find this approach a bit lame, because 
milestone itself is not a proper task, it has nothing else but title, 
description, and due date. Also, it's probably wrong to call this entity a 
milestone.

>
>>  3. Sub-par attachments
>>  --
>>
>>  Traditional bug trackers allow attaching files to issue. GitHub goes 
>> further and allows to attach files to every comment. Enjoy the progress - 
>> now you can look for attached test cases and proposed patches through all 
>> comment feed, instead of having them in one place at the top.
>>
>>  Also, on test cases. You probably like this feature of Bugzilla when you 
>> can attach self-contained HTML file to the issue and then simply open it by 
>> URL in any browser including your build of WebKit to try it out. Forget this 
>> - GitHub simply forbids HTML or JS attachments (without wrapping them in 
>> archive):
>>
>>  "We don’t support that file type. with a GIF, JPEG, JPG, PNG, DOCX, GZ, 
>> LOG, PDF, PPTX, TXT, XLSX or ZIP."
>>
>>  And yes, take care not to use tar.xz or tar.bz2 or any other unapproved 
>> archive type.
>>
>>  But you can attach funny cat picture to your comment and it will be 
>> displayed inline :)
>
> This is another massive functional regression. I open test cases on
> Bugzilla without downloading all the time, not to mention that it's a
> great way to test iOS devices as well. Not being able to do that would
> significantly reduce my productivity.
>
> - R. Niwa


-- 
Regards,
Konstantin
___
webkit-dev mailing list
webkit-dev@lists.

Re: [webkit-dev] WebKit Transition to Git

2020-10-06 Thread Ryosuke Niwa

On Mon, Oct 5, 2020 at 5:13 PM Konstantin Tokarev  wrote:
>
>
> 05.10.2020, 23:41, "Yusuke Suzuki" :
> > I think security component is special in terms of how to handle it already 
> > (e.g. not posting a test with the patch etc.)
> > To me, handling non-security issues in GitHub and security issues in 
> > Bugzilla is OK.
> > By nature, security issues are not open. Since one of our motivation of 
> > moving to GitHub is openness for feedback collection, security issue in 
> > Bugzilla does not matter for this motivation.
> > Ideally, handling both in GitHub is better. But to me, rather than 
> > continuing using Bugzilla, using GitHub for non security issues sounds 
> > improvement.
>
> To me it sounds as a huge step backwards. Asides from situation with security 
> issues, it has other significant drawbacks in domain of issue triaging and 
> management:
>
> 1. Sub-par support for linking issues to each other
> 
>
> Traditional bug tracking systems (like Bugzilla or JIRA) have support of 
> "related" or "linked" issues. Most important relations are
>
> * A depends on B (B blocks A) - blockers and umbrella issues
> * B is duplicate of A
> * A and B are related in other unspecified way
>
> All GitHub can offer here now is mentions (and, to some extent, milestones 
> for case of "umbrella issues" [1]). Mention is created every time someone 
> uses "#" (e.g. "#123") in the text of issue or in the comment, where 
> number is a sequential number of issue or pull request [2]. When comment is 
> published in issue A which mentions issue B, there is a pseudo-comment added 
> to B, and subscribers of B receive email notification.
>
> At first glance this naive approach seems to work, but
>
> * There is no easily visible list of relations: if you are not closely 
> following all activity on A, to find all issues related to it you have to 
> browse through all its (pseudo-)comments, which in some cases might be long.
> * There is no *stateful* list of relations: if A was believed to have common 
> source with B, but then it was discovered they are not related, you cannot 
> delete relationship between A and B because there is no relationship, just a 
> set of comments.
> * "#" is not a safe reference format. Sometimes users' comments may 
> have other data in "#" format with a different meaning than 
> references to GitHub issues. For example, may the force be with you if 
> someone pastes gdb or lldb backtrace into comment without escaping it into 
> markdown raw text block (```). Also, GitHub parses mentions in git commit 
> messages, so care must be taken to avoid any occurrences of "#" with 
> a meaning different from reference to issue number.

Yeah, this is a pretty significant functional regression to me. I use
bug dependencies all the time (e.g.
https://bugs.webkit.org/showdependencytree.cgi?id=148695&hide_resolved=1)
and not having this capability will significantly hinder my ability to
track & triage some bugs.

> 3. Sub-par attachments
> --
>
> Traditional bug trackers allow attaching files to issue. GitHub goes further 
> and allows to attach files to every comment. Enjoy the progress -  now you 
> can look for attached test cases and proposed patches through all comment 
> feed, instead of having them in one place at the top.
>
> Also, on test cases. You probably like this feature of Bugzilla when you can 
> attach self-contained HTML file to the issue and then simply open it by URL 
> in any browser including your build of WebKit to try it out. Forget this - 
> GitHub simply forbids HTML or JS attachments (without wrapping them in 
> archive):
>
> "We don’t support that file type. with a GIF, JPEG, JPG, PNG, DOCX, GZ, 
> LOG, PDF, PPTX, TXT, XLSX or ZIP."
>
> And yes, take care not to use tar.xz or tar.bz2 or any other unapproved 
> archive type.
>
> But you can attach funny cat picture to your comment and it will be displayed 
> inline :)

This is another massive functional regression. I open test cases on
Bugzilla without downloading all the time, not to mention that it's a
great way to test iOS devices as well. Not being able to do that would
significantly reduce my productivity.

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

Re: [webkit-dev] WebKit Transition to Git

2020-10-05 Thread Konstantin Tokarev



05.10.2020, 13:26, "Frédéric Wang" :
> One thing to take into account is that WebKit's repository is big and
> public GitHub/GitLab prevent creating large repository by default. This
> means it might not be possible for contributors to actually fork
> WebKit's repository on their account and then create a pull request
> (which is the standard way IIUC).

Every repository which is already present on GitHub can be forked by
anyone without any restrictions.

There are restrictions for repositories which are not forks, e.g. if you
create new repo on your account and try to push whole WebKit there,
you'll get an error.

> Instead, we would probably end up
> doing like web-platform-tests and give contributors the permission to
> create branches to the WebKit account and make Pull Request to the
> master branch. 

This is not needed and should be avoided - private development branches
in main repo will confuse users and will be copied into all forks.


>Probably, we should forbid people to commit to the master
> branch directly (I think someone broke WPT's master branch that way last
> year)...

I have to admit that ability to make small unreviewed commit for thing like 
typo fix or change in personal watchlist is rather useful...

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

Re: [webkit-dev] WebKit Transition to Git

2020-10-05 Thread Konstantin Tokarev



05.10.2020, 17:19, "Robert Ma" :
> On Mon, Oct 5, 2020 at 6:22 AM Frédéric Wang  wrote:
>> One thing to take into account is that WebKit's repository is big and
>> public GitHub/GitLab prevent creating large repository by default. This
>> means it might not be possible for contributors to actually fork
>> WebKit's repository on their account and then create a pull request
>> (which is the standard way IIUC). Instead, we would probably end up
>> doing like web-platform-tests and give contributors the permission to
>> create branches to the WebKit account and make Pull Request to the
>> master branch. Probably, we should forbid people to commit to the master
>> branch directly (I think someone broke WPT's master branch that way last
>> year)...
>
> Note that this is not exactly the reason we give people write access in WPT. 
> The WPT repo is not too big to impede forking. Rather, this was to work 
> around some CI setup constraints (e.g. some secrets not accessible from 
> forks), most of which have been resolved so the write access is largely for 
> convenience for active developers (e.g. avoid having to sync the fork).

There is no need to sync the fork for any development activity. You can have 
WPT repo as an upstream for master branch, and push your topic branches to your 
own fork added as another remote. When you pull master it will be updated from 
WPT repo, or you can do git fetch and rebase your topic branch to origin/master.

The only possible downside is that master branch of your fork may become 
outdated, but since you don't need it for anything and use local master which 
is always in sync, it's just cosmetic issue.

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

Re: [webkit-dev] WebKit Transition to Git

2020-10-05 Thread Konstantin Tokarev



05.10.2020, 23:41, "Yusuke Suzuki" :
> I think security component is special in terms of how to handle it already 
> (e.g. not posting a test with the patch etc.)
> To me, handling non-security issues in GitHub and security issues in Bugzilla 
> is OK.
> By nature, security issues are not open. Since one of our motivation of 
> moving to GitHub is openness for feedback collection, security issue in 
> Bugzilla does not matter for this motivation.
> Ideally, handling both in GitHub is better. But to me, rather than continuing 
> using Bugzilla, using GitHub for non security issues sounds improvement.

To me it sounds as a huge step backwards. Asides from situation with security 
issues, it has other significant drawbacks in domain of issue triaging and 
management:

1. Sub-par support for linking issues to each other


Traditional bug tracking systems (like Bugzilla or JIRA) have support of 
"related" or "linked" issues. Most important relations are

* A depends on B (B blocks A) - blockers and umbrella issues
* B is duplicate of A
* A and B are related in other unspecified way

All GitHub can offer here now is mentions (and, to some extent, milestones for 
case of "umbrella issues" [1]). Mention is created every time someone uses 
"#" (e.g. "#123") in the text of issue or in the comment, where number 
is a sequential number of issue or pull request [2]. When comment is published 
in issue A which mentions issue B, there is a pseudo-comment added to B, and 
subscribers of B receive email notification.

At first glance this naive approach seems to work, but

* There is no easily visible list of relations: if you are not closely 
following all activity on A, to find all issues related to it you have to 
browse through all its (pseudo-)comments, which in some cases might be long. 
* There is no *stateful* list of relations: if A was believed to have common 
source with B, but then it was discovered they are not related, you cannot 
delete relationship between A and B because there is no relationship, just a 
set of comments.
* "#" is not a safe reference format. Sometimes users' comments may 
have other data in "#" format with a different meaning than references 
to GitHub issues. For example, may the force be with you if someone pastes gdb 
or lldb backtrace into comment without escaping it into markdown raw text block 
(```). Also, GitHub parses mentions in git commit messages, so care must be 
taken to avoid any occurrences of "#" with a meaning different from 
reference to issue number.

---

[1] Milestones are not issues themselves, but they establish true two-way 
stateful relation between issues and their "parent" milestone.
[2] For some reason they have shared numbering, which means sometimes you may 
not know what kind of reference is in front of you until you look up its URL or 
navigate


2. Sub-par issue categorization and search
--

In traditional bug tracking systems every issue has properties like status, 
resolution, component, severity/issue type, priority. When new bug is created, 
user chooses values of these properties, which can be later adjusted by the 
person doing triaging. They also are used in user-friendly search dialog like 
[1].

All GitHub can offer here are custom labels. While in theory they are 
sufficient to replace pre-defined issue properties, they require disciplined 
use to be efficient, for example issue must not have mutually exclusive labels. 
To avoid chaos, GitHub allows setting issue labels only to contributors. 
However, this puts more work on triagers, and they cannot triage only certain 
kinds of issues which match their interests by going through results of search 
query.

[1] https://bugs.webkit.org/query.cgi


3. Sub-par attachments
--

Traditional bug trackers allow attaching files to issue. GitHub goes further 
and allows to attach files to every comment. Enjoy the progress -  now you can 
look for attached test cases and proposed patches through all comment feed, 
instead of having them in one place at the top.

Also, on test cases. You probably like this feature of Bugzilla when you can 
attach self-contained HTML file to the issue and then simply open it by URL in 
any browser including your build of WebKit to try it out. Forget this - GitHub 
simply forbids HTML or JS attachments (without wrapping them in archive):

"We don’t support that file type. with a GIF, JPEG, JPG, PNG, DOCX, GZ, 
LOG, PDF, PPTX, TXT, XLSX or ZIP."

And yes, take care not to use tar.xz or tar.bz2 or any other unapproved archive 
type.

But you can attach funny cat picture to your comment and it will be displayed 
inline :)


Conclusion
--

You can say this is all small issues which are not really significant. With 
current state of things when Bugzilla is mostly used as a code review tool, and 
very few of issues reported by p

Re: [webkit-dev] WebKit Transition to Git

2020-10-05 Thread Yusuke Suzuki

I think security component is special in terms of how to handle it already 
(e.g. not posting a test with the patch etc.)
To me, handling non-security issues in GitHub and security issues in Bugzilla 
is OK.
By nature, security issues are not open. Since one of our motivation of moving 
to GitHub is openness for feedback collection, security issue in Bugzilla does 
not matter for this motivation.
Ideally, handling both in GitHub is better. But to me, rather than continuing 
using Bugzilla, using GitHub for non security issues sounds improvement.

-Yusuke

> On Oct 5, 2020, at 12:58 PM, Fujii Hironori  wrote:
> 
> 
> 
> On Tue, Oct 6, 2020 at 12:40 AM Jonathan Bedard  > wrote:
> 
> That's one solution, but even that is somewhat insufficient because we don’t 
> want to give someone access to every security issue just to give access to a 
> single one. One of the solutions we’ve discussed is to migrate bugs component 
> by component, the security component may stay on bugzilla indefinitely.
> 
> 
>  I don't like this idea of using GitHub issue tracker and Bugzilla together. 
> Let's keep using Bugzilla for all components.
> ___
> 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] WebKit Transition to Git

2020-10-05 Thread Fujii Hironori

On Tue, Oct 6, 2020 at 12:40 AM Jonathan Bedard  wrote:

>
> That's one solution, but even that is somewhat insufficient because we
> don’t want to give someone access to every security issue just to give
> access to a single one. One of the solutions we’ve discussed is to migrate
> bugs component by component, the security component may stay on bugzilla
> indefinitely.
>
>
 I don't like this idea of using GitHub issue tracker and Bugzilla
together. Let's keep using Bugzilla for all components.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebKit Transition to Git

2020-10-05 Thread Michael Catanzaro

On Mon, Oct 5, 2020 at 8:40 am, Jonathan Bedard  
wrote:
That's one solution, but even that is somewhat insufficient because 
we don’t want to give someone access to every security issue just 
to give access to a single one. One of the solutions we’ve 
discussed is to migrate bugs component by component, the security 
component may stay on bugzilla indefinitely.


Can you grant access to a single issue by CCing the desired user to the 
issue? I expect that would work (not tested).



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

Re: [webkit-dev] WebKit Transition to Git [Identifiers and Merge Commits]

2020-10-05 Thread Jonathan Bedard


>> Monotonic Commit Identifiers
>> Of great interest to Apple?s engineers has been retaining some kind
>> of ordered tag we can use to refer to commits to make defending CI
>> and bisection easier. We?ve developed a scheme for this that assigns
>> commits an ordered identifier per-branch, outlined in?
>> https://trac.webkit.org/wiki/commit-identifiers 
>> , designed to be used
>> alongside git hashes. These identifiers can be used in our current
>> Subversion repository, and we would like to start using them before
>> the project has transitions to git.
>> 
> 
> Have you seen how this was handled in LLVM?
> https://releases.llvm.org/9.0.0/docs/Proposals/GitHubMove.html#on-managing-revision-numbers-with-git
>  
> 

We looked at LLVM and Chromium specifically, our approach is pretty similar to 
LLVM’s, but resets the canonical identifiers when branching from the default 
branch.

> 
> Would you also consider preventing merge commits in order to keep a
> clean mainline branch?


Definitely going to be preventing merge commits, they end up breaking a lot, I 
neglected to mention them because I often (perhaps out of wishful thinking) 
forget they exist.

Jonathan

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

Re: [webkit-dev] WebKit Transition to Git

2020-10-05 Thread Jonathan Bedard

> On Oct 2, 2020, at 5:05 PM, Michael Catanzaro  wrote:
> 
> On Fri, Oct 2, 2020 at 09:43, Jonathan Bedard  wrote:
>> The biggest blocker we are aware of is managing security bugs, since the 
>> security advisory system used by GitHub is essentially the opposite of how 
>> WebKit security bugs work. Moving to GitHub Issues, if it happens, will be 
>> the last part of this transition, and we are interested in soliciting 
>> feedback from our contributors on what the WebKit project´s integration with 
>> GitHub Issues should look like.
> 
> I don't think we need much integration to use the issue tracker? Once we 
> migrate existing bugs from WebKit Bugzilla, we can use it as we would any 
> other issue tracker? Why would it require integration?

We have some tooling attached to bugzilla (webkit-patch is the highest profile, 
but there are a few others), that’s the sort of thing I’m referring to.

> 
> We might need to use a separate repository with more limited permissions to 
> handle security reports. At least in GitLab, all project developers 
> (committers) have access to all confidential issues. I'm not sure about 
> GitHub, but I assume it would be the same.

That's one solution, but even that is somewhat insufficient because we don’t 
want to give someone access to every security issue just to give access to a 
single one. One of the solutions we’ve discussed is to migrate bugs component 
by component, the security component may stay on bugzilla indefinitely.

> 
> What will require integration is pull request merges. If we want to maintain 
> linear version history, we will want a merge bot. On GNOME GitLab, we have a 
> large number of smaller projects and it's we don't need them, but for a one 
> huge project like WebKit there will be too many conflicts otherwise, because 
> every commit going into the main branch will require all other pull requests 
> to be rebased. A merge bot -- e.g. [1] -- will handle that for us. (Not sure 
> what merge bots are common on GitHub. )

We definitely will have a merge bot (basically what the current commit queue 
is). They are pretty common in GitHub, especially because they’re a good way to 
manage access to protected branches. Aakash and I have discussed some of the 
specifics, the #github-migration channel on Slack is going to be the best place 
to discuss the details of this.

> 
> Michael
> 
> [1] https://gitlab.com/fsdk-marge-bot
> 
> 

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

Re: [webkit-dev] WebKit Transition to Git

2020-10-05 Thread Jonathan Bedard

`git describe` is quite a bit less powerful, unfortunately. First, while the 
string coming from `git-describe` can be used in git commands, that’s only 
because it contains the abbreviated commit hash, which alone is sufficient to 
identify a commit. Second, such identifiers end up being pretty weird on the 
default branch since we often use tags to mark branch points. This means that 
commits on the default branch would reset their count every time we branched.

Jonathan

> On Oct 4, 2020, at 2:41 PM, Konstantin Tokarev  wrote:
> 
> 
> 
> 02.10.2020, 19:46, "Jonathan Bedard" :
>> Monotonic Commit Identifiers
>> Of great interest to Apple’s engineers has been retaining some kind of 
>> ordered tag we can use to refer to commits to make defending CI and 
>> bisection easier. We’ve developed a scheme for this that assigns commits an 
>> ordered identifier per-branch, outlined in 
>> https://trac.webkit.org/wiki/commit-identifiers, designed to be used 
>> alongside git hashes. These identifiers can be used in our current 
>> Subversion repository, and we would like to start using them before the 
>> project has transitions to git.
> 
> AFAIU, this is very close to what `git describe` does: you give it git hash, 
> it gives you new identifier consisting of 3 parts:
> 
> --
> 
> Note that resulting identifier can be used in all git operations which 
> require git reference (like `git log` or `git show`).
> So, if you push git tags to main repository (maybe lightweight tags, if you 
> are planning to have lots of them), there is no need to invent any other 
> identifiers.
> 
> As for bisection, git bisect works just fine when given two commit hashes.
> 
> 
> -- 
> Regards,
> Konstantin

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

Re: [webkit-dev] WebKit Transition to Git

2020-10-05 Thread Frédéric Wang

On 05/10/2020 16:15, Robert Ma wrote:
> On Mon, Oct 5, 2020 at 6:22 AM Frédéric Wang  > wrote:
>
> One thing to take into account is that WebKit's repository is big and
> public GitHub/GitLab prevent creating large repository by default.
> This
> means it might not be possible for contributors to actually fork
> WebKit's repository on their account and then create a pull request
> (which is the standard way IIUC). Instead, we would probably end up
> doing like web-platform-tests and give contributors the permission to
> create branches to the WebKit account and make Pull Request to the
> master branch. Probably, we should forbid people to commit to the
> master
> branch directly (I think someone broke WPT's master branch that
> way last
> year)...
>
>  
> Note that this is not exactly the reason we give people write access
> in WPT. The WPT repo is not too big to impede forking. Rather, this
> was to work around some CI setup constraints (e.g. some secrets not
> accessible from forks), most of which have been resolved so the write
> access is largely for convenience for active developers (e.g. avoid
> having to sync the fork
> ).
>
Right, I didn't mean that but thanks for clarifying Robert (and for
adding more info about potential issues with this PR/fork-based workflow).

-- 
Frédéric Wang

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

Re: [webkit-dev] WebKit Transition to Git

2020-10-05 Thread Adrien Destugues

> One thing to take into account is that WebKit's repository is big and
> public GitHub/GitLab prevent creating large repository by default. This
> means it might not be possible for contributors to actually fork
> WebKit's repository on their account and then create a pull request
> (which is the standard way IIUC). Instead, we would probably end up
> doing like web-platform-tests and give contributors the permission to
> create branches to the WebKit account and make Pull Request to the
> master branch. Probably, we should forbid people to commit to the master
> branch directly (I think someone broke WPT's master branch that way last
> year)...
> 
> Similarly, public GitHub/GitLab prevent very large files by default.
> Hopefully, this does not happen in WebKit (we should definitely add a
> hook to prevent someone to land a big file) but for example that mistake
> happened in Chromium last year (
> https://groups.google.com/u/1/a/chromium.org/g/blink-dev/c/tSOmIfXAP2s/m/H07SmWIoCgAJ
> ) and this broke the sync of their GitHub mirror.

There is already a mirror of the existing WebKit repository on Github, and 
there are
about 2000 forks of it already [1]. At least the Haiku version of WebKit is 
maintained this
way, and had no problems with Github so far. So this shouldn't be a problem?

[1] https://github.com/WebKit/webkit/network/members

-- 
Adrien.

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

Re: [webkit-dev] WebKit Transition to Git

2020-10-05 Thread Robert Ma

On Mon, Oct 5, 2020 at 6:22 AM Frédéric Wang  wrote:

> One thing to take into account is that WebKit's repository is big and
> public GitHub/GitLab prevent creating large repository by default. This
> means it might not be possible for contributors to actually fork
> WebKit's repository on their account and then create a pull request
> (which is the standard way IIUC). Instead, we would probably end up
> doing like web-platform-tests and give contributors the permission to
> create branches to the WebKit account and make Pull Request to the
> master branch. Probably, we should forbid people to commit to the master
> branch directly (I think someone broke WPT's master branch that way last
> year)...
>

Note that this is not exactly the reason we give people write access in
WPT. The WPT repo is not too big to impede forking. Rather, this was to
work around some CI setup constraints (e.g. some secrets not accessible
from forks), most of which have been resolved so the write access is
largely for convenience for active developers (e.g. avoid having to sync
the fork

).

And yeah, do remember to protect the master branch :)
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebKit Transition to Git

2020-10-05 Thread Frédéric Wang

On 02/10/2020 18:43, Jonathan Bedard wrote:
Hi all,

This is great to hear! Several people have already made comments about
things that I wanted to say, but I'd just to add my personal point of
view...

I've used both Mozilla's Phabricator and Chromium's Gerrit for
patches/review, and I find both of them great and pretty easy to use. Of
course it can take time to get familiar with the UI, but I don't think
this is a big blocker, the same is true with any tool...

Mozilla and Chromium projects are basically using a separate tracker for
bug reports and a Phabricator's differential / Gerrit's CL are connected
to a bug entry. Mozilla's differential are attached to Bugzilla as a
patch (which is good to make people cc'ed to the bug aware of WIP patch)
while Chromium's Gerrit only posts the commit message when the patch lands.

Both tools generate a increasing ID in the commit message. However, I
think Phabricator's ID is done when the differential is created, not at
commit time so this is not what we want for WebKit I guess. In any case,
I don't think there should be a problem to add a hook to ensure the ID
is created when landing the patch.

When you upload a patch you are basically creating a new differential/CL
or updating an existing one with a new revision and the difference
between revision is calculated automatically. To upload a patch, you
execute a script similar to our "webkit-patch upload" which can do extra
checks (e.g. style, suggesting reviewers, etc). In any case, Mozilla and
Chromium's projects perform these checks when the patch is uploaded too.

I'm not sure for Mozilla's phabricator but in Chromium's Gerrit you can
also make a CL depend on another one (I mean the review tool and bots
will understand that dependency and behave "nicely"), which is helpful
to split a change into several patches.

I personally prefer both of them over GitHub/GitLab's approach with
branches & merge/pull requests. That way you don't have to publish your
branches / rebases / forced pushes / squashes, etc Instead, you would do
whatever you want in your local clone & branches and always only send
the relevant diff as a new revision. Others say similar features are
available/planned for GitHub/GitLab, so maybe it's not a problem.

One thing to take into account is that WebKit's repository is big and
public GitHub/GitLab prevent creating large repository by default. This
means it might not be possible for contributors to actually fork
WebKit's repository on their account and then create a pull request
(which is the standard way IIUC). Instead, we would probably end up
doing like web-platform-tests and give contributors the permission to
create branches to the WebKit account and make Pull Request to the
master branch. Probably, we should forbid people to commit to the master
branch directly (I think someone broke WPT's master branch that way last
year)...

Similarly, public GitHub/GitLab prevent very large files by default.
Hopefully, this does not happen in WebKit (we should definitely add a
hook to prevent someone to land a big file) but for example that mistake
happened in Chromium last year (
https://groups.google.com/u/1/a/chromium.org/g/blink-dev/c/tSOmIfXAP2s/m/H07SmWIoCgAJ
) and this broke the sync of their GitHub mirror.

I don't know the details of the conversation between Google & GitHub,
but it took several weeks and at the end at Igalia we decided to switch
our MathML development branch to an internal GitLab repository on which
we have full control. Apple might want to weight the pros/cons of
managing their own instance VS relying on proprietary / third-party
instances for this kind of issue or other customizations.

In general, I personally believe it would be good if one only require to
use free software tool to contribute to WebKit. But I guess we already
made an exception by switching to Slack, so...

-- 
Frédéric Wang

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

Re: [webkit-dev] WebKit Transition to Git

2020-10-04 Thread Ryosuke Niwa

On Sun, Oct 4, 2020 at 2:41 PM Konstantin Tokarev  wrote:
>
> 02.10.2020, 19:46, "Jonathan Bedard" :
> > Monotonic Commit Identifiers
> > Of great interest to Apple’s engineers has been retaining some kind of 
> > ordered tag we can use to refer to commits to make defending CI and 
> > bisection easier. We’ve developed a scheme for this that assigns commits an 
> > ordered identifier per-branch, outlined in 
> > https://trac.webkit.org/wiki/commit-identifiers, designed to be used 
> > alongside git hashes. These identifiers can be used in our current 
> > Subversion repository, and we would like to start using them before the 
> > project has transitions to git.
>
> AFAIU, this is very close to what `git describe` does: you give it git hash, 
> it gives you new identifier consisting of 3 parts:
>
> --

We don't want to be using a number from the closest tag. We need a
contiguous monotonically increasing number on the main branch.

> Note that resulting identifier can be used in all git operations which 
> require git reference (like `git log` or `git show`).
> So, if you push git tags to main repository (maybe lightweight tags, if you 
> are planning to have lots of them), there is no need to invent any other 
> identifiers.
>
> As for bisection, git bisect works just fine when given two commit hashes.

"git bisect" is wholly inadequate for our purposes. Not only can
building between each step can take ages (15-40min depending on a
machine) even for correctness bisections, some of our performance
tests will require many hours to measure statistically significant
results. On top of that, many of our bisections can take weeks if not
months to conduct and could span hundreds of commits in between. In
some cases, we have to compare time series charts of performance data
across multiple operating systems and device classes. Furthermore, we
have to communicate all this information between different teams
working on the bisection, someone who wrote the original patch, QA
people testing various builds, etc... It's extremely important that we
have a **human readable number** which is easy to memorize and intuit
during these analyses and communications.

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

Re: [webkit-dev] WebKit Transition to Git

2020-10-04 Thread Konstantin Tokarev



02.10.2020, 19:46, "Jonathan Bedard" :
> Monotonic Commit Identifiers
> Of great interest to Apple’s engineers has been retaining some kind of 
> ordered tag we can use to refer to commits to make defending CI and bisection 
> easier. We’ve developed a scheme for this that assigns commits an ordered 
> identifier per-branch, outlined in 
> https://trac.webkit.org/wiki/commit-identifiers, designed to be used 
> alongside git hashes. These identifiers can be used in our current Subversion 
> repository, and we would like to start using them before the project has 
> transitions to git.

AFAIU, this is very close to what `git describe` does: you give it git hash, it 
gives you new identifier consisting of 3 parts:

--

Note that resulting identifier can be used in all git operations which require 
git reference (like `git log` or `git show`).
So, if you push git tags to main repository (maybe lightweight tags, if you are 
planning to have lots of them), there is no need to invent any other 
identifiers.

As for bisection, git bisect works just fine when given two commit hashes.


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

Re: [webkit-dev] WebKit Transition to Git

2020-10-04 Thread Yusuke Suzuki

GitHub is planning to have review revisions in 2020/Q4.
So, when WebKit repository is migrated, we already have that in GitHub builtin 
review tool :)

https://github.com/github/roadmap/issues/54 


Best regards,
Yusuke

> On Oct 3, 2020, at 5:18 PM, Michael Catanzaro  wrote:
> 
> On Sat, Oct 3, 2020 at 9:52 pm, Konstantin Tokarev  wrote:
>> If I understand correctly, there is no way in GitHub UI to see a difference 
>> between
>> patches submitted at step 1 and step 3. It's still possible to see old 
>> version of patches
>> if you navigate to old commit hash, but that's not for long as they can be 
>> garbage
>> collected by GitHub because they don't belong to git branch anymore.
> 
> If we are really seriously concerned about this... it's not a problem with 
> GitLab. At least, I can still view diffs between different revisions of merge 
> requests from two years ago, and the changes seem to be stored forever. At 
> least I still can view diffs from years ago. E.g.:
> 
> https://gitlab.gnome.org/GNOME/epiphany/-/merge_requests/62/diffs?diff_id=27669&start_sha=0c020384b602c9e1f0e8ec9e491d1951e8feadf7
> 
> Unfortunately it's sometimes less useful than I might have hoped, because 
> rebases bring in a bunch of totally unrelated changes, e.g.:
> 
> https://gitlab.gnome.org/GNOME/epiphany/-/merge_requests/62/diffs?diff_id=28036&start_sha=043b5fc32f4f9263d393c9de83e1b33123c5
> 
> 
> 
> ___
> 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] WebKit Transition to Git

2020-10-03 Thread Michael Catanzaro

On Sat, Oct 3, 2020 at 9:52 pm, Konstantin Tokarev  
wrote:
If I understand correctly, there is no way in GitHub UI to see a 
difference between
patches submitted at step 1 and step 3. It's still possible to see 
old version of patches
if you navigate to old commit hash, but that's not for long as they 
can be garbage

collected by GitHub because they don't belong to git branch anymore.


If we are really seriously concerned about this... it's not a problem 
with GitLab. At least, I can still view diffs between different 
revisions of merge requests from two years ago, and the changes seem to 
be stored forever. At least I still can view diffs from years ago. E.g.:


https://gitlab.gnome.org/GNOME/epiphany/-/merge_requests/62/diffs?diff_id=27669&start_sha=0c020384b602c9e1f0e8ec9e491d1951e8feadf7

Unfortunately it's sometimes less useful than I might have hoped, 
because rebases bring in a bunch of totally unrelated changes, e.g.:


https://gitlab.gnome.org/GNOME/epiphany/-/merge_requests/62/diffs?diff_id=28036&start_sha=043b5fc32f4f9263d393c9de83e1b33123c5



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

Re: [webkit-dev] WebKit Transition to Git



03.10.2020, 21:49, "Adrien Destugues" :
> Yes that's why I didn't elaborate much. Whichever tool you pick, there
> will always be people unhappy about it.

Right. For example, I have negative bias against GitLab, because this
company have bought its open source competitor (Gitorious, which was
quite popular service those days) just to shut it down.

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

Re: [webkit-dev] WebKit Transition to Git

02.10.2020, 23:17, "Jonathan Bedard" :
>>  On Oct 2, 2020, at 11:47 AM, Tetsuharu OHZEKI  
>> wrote:
>>
>>  Hi Jonathan,
>>
>>  As a contributor, I hear this change positively and I'm looking
>>  forward to transition to a new process.
>>
>>  I have some questions and feelings:
>>
>>  1. Will we continue to use https://trac.webkit.org/wiki after moving
>>  to something to host the Git repository?
>>
>>  This is just my curiosity.
>
> We don’t have plans to migrate the wiki in the immediate future, although I 
> suspect that as development workflows switch to GitHub, we will eventually 
> want to move the wiki to GitHub as well.

If you are considering this, please take into account that GitHub doesn't allow
to configure separate access lists for repository and wiki. There are only two
options:

1) wiki can be edited by any GitHub user
2) wiki can be edited by people who have write access to repo (i.e. right to 
make
unreviewed push to any branch, as there are no "partial access" settings)

It's not possible to have a separate list of "contributors" without push right 
as
we do now.

Of course, this can be worked around via setting up wiki in a different 
repository
with different access list.

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

Re: [webkit-dev] WebKit Transition to Git



03.10.2020, 05:18, "Ryosuke Niwa" :
> On Fri, Oct 2, 2020 at 5:06 PM Michael Catanzaro  wrote:
>>  On Fri, Oct 2, 2020 at 13:48, Ken Russell  wrote:
>>  > Github's code review UI has a couple of feature gaps in my opinion.
>>  > It's difficult to look at earlier versions of the pull request, in
>>  > particular to verify that issues found during code review have been
>>  > fixed. I remember it also being difficult to figure out whether all
>>  > comments on earlier versions have been addressed.
>>
>>  I'm not familiar with reviews on GitHub. I just want to add that GitLab
>>  reviews are great and have none of these problems. It's very easy to
>>  view older versions of merge requests and compare the differences
>>  between arbitrary revisions of the merge request. (That's often
>>  important when reviewing small changes to a large merge request.) Every
>>  discussion thread is clearly marked as either resolved (green!) or
>>  unresolved, and you can (and should) configure GitLab to block merges
>>  until all discussions are resolved. (I would be disappointed if GitHub
>>  doesn't have the same convenience features, as this helps prevent
>>  mistakes from being forgotten.)
>
> GitHub totally offers this. See, for example:
> https://github.com/whatwg/html/pull/5912

Let's consider this example in depth.

1. mfreed7 have pushed a patch series 
2. You and others have added comments on what should be changed in patches
3. mfreed7 did all the changes in patches and force-pushed result

If I understand correctly, there is no way in GitHub UI to see a difference 
between
patches submitted at step 1 and step 3. It's still possible to see old version 
of patches
if you navigate to old commit hash, but that's not for long as they can be 
garbage
collected by GitHub because they don't belong to git branch anymore.

In contrast, with Gerrit you can see all revisions of each patch and compare
arbitrary revisions side by side to see how your comments were addressed and
what else was changed.

>
>>  > 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.)
>
> Indeed. Gerrit's UI is also quite dense and hard to use for someone
> new to it too:
> https://gerrit-review.googlesource.com/c/plugins/replication/+/282176
>
> - R. Niwa
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev


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

Re: [webkit-dev] WebKit Transition to Git

2020-10-03 Thread Adrien Destugues

On Sat, Oct 03, 2020 at 08:43:40PM +0300, Konstantin Tokarev wrote:
> >>  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.
> >
> > What are the reasons why people don't want to have a GitHub account? I 
> > wonder if that's somehow a different kind of barrier to entry to be 
> > concerned about.
> 
> Maybe because it's owned by Microsoft, which is an enemy of all living things 
> and
> basically are an embassy of Devil on Earth (no).

Yes that's why I didn't elaborate much. Whichever tool you pick, there
will always be people unhappy about it.

In the case of Github, reasons I've heard of are (not that I agree or
disagree with anything here, that's not the point):
- It's not open source
- It's owned by Microsoft
- It has contracts with the ICE in the USA
  
(https://www.theatlantic.com/technology/archive/2020/01/ice-contract-github-sparks-developer-protests/604339/)
- It's too big and making a large part of open source work centralized in one
  place (mainly from people who already suffered from problems with Sourceforge
  or Google Code Project Hosting in the past, I guess)

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

Re: [webkit-dev] WebKit Transition to Git



03.10.2020, 20:40, "Michael Catanzaro" :
> On Sat, Oct 3, 2020 at 3:16 am, Ryosuke Niwa  wrote:
>>  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.
>
> You may not be able to add inline comments on commit messages, but I've
> never been particularly concerned about that. You can still start a new
> discussion thread mentioning the problem with the commit message that
> you'd like to see resolved, blocking merge until the discussion thread
> is resolved. Although in GNOME we don't often have problems with
> low-quality commit messages, we do sometimes, and during review we
> treat that as we would any problem with the code. Having a set of
> guidelines for writing commit messages, like [1], might help. But yes,
> we do lose the ability to do inline comments during code review.

FWIW, Gerrit allows to do inline comments on commit messages,
or to add comment for the whole file.

>
> (Anyway, this is only a tangent, since of course we can switch to
> GitHub but still keep ChangeLog files if we decided to do that.)
>
> [1] https://wiki.gnome.org/Git/CommitMessages
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev


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

Re: [webkit-dev] WebKit Transition to Git



03.10.2020, 19:34, "Alexey Proskuryakov" :
>>  3 окт. 2020 г., в 2:24 AM, Adrien Destugues  
>> написал(а):
>>
>>  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.
>
> What are the reasons why people don't want to have a GitHub account? I wonder 
> if that's somehow a different kind of barrier to entry to be concerned about.

Maybe because it's owned by Microsoft, which is an enemy of all living things 
and
basically are an embassy of Devil on Earth (no).

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

Re: [webkit-dev] WebKit Transition to Git

2020-10-03 Thread Michael Catanzaro


On Sat, Oct 3, 2020 at 3:16 am, Ryosuke Niwa  wrote:

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.


You may not be able to add inline comments on commit messages, but I've 
never been particularly concerned about that. You can still start a new 
discussion thread mentioning the problem with the commit message that 
you'd like to see resolved, blocking merge until the discussion thread 
is resolved. Although in GNOME we don't often have problems with 
low-quality commit messages, we do sometimes, and during review we 
treat that as we would any problem with the code. Having a set of 
guidelines for writing commit messages, like [1], might help. But yes, 
we do lose the ability to do inline comments during code review.


(Anyway, this is only a tangent, since of course we can switch to 
GitHub but still keep ChangeLog files if we decided to do that.)


[1] https://wiki.gnome.org/Git/CommitMessages


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

Re: [webkit-dev] WebKit Transition to Git