RFC: Moving git-gui development to GitHub

RFC: Moving git-gui development to GitHub

[Pratyush Yadav]

Hi everyone,

I recently had some discussions with Dscho about whether it is a better 
idea to use GitHub for development instead of email [0]. His argument 
was that GitHub makes it easier for newcomers to contribute, since more 
people are familiar with GitHub compared to mailing lists. Also, it is 
somewhat difficult to set up an email-based workflow.

A pretty good argument. Using GitHub would certainly make one-off 
contributions easier, and make it easier for newcomers to get involved.

But I feel like it is equally important to know what is good for the 
long-term contributors. Since I've been involved with git-gui for a 
relatively short time, I don't know many long term active contributors. 
Those I know of are in Cc. These are people who have frequently or 
semi-frequently expressed interest in git-gui development. Of course, 
people not in Cc are also welcome to express their opinions, and if I 
forgot to put someone in there that I should have, my apologies.

I want to know people's opinion on whether it is a good idea to move 
development to GitHub.

Just to lay out my views on the subject, here's a list of advantages and 
disadvantages that I can come up with.

Arguments in favor of moving to GitHub:

- Easier for new and one-off contributors.

- Potentially easier for existing contributors. A lot of people are 
  already very comfortable with GitHub's workflow.

- The "Issues" section can serve as an issue tracker. Right now, my 
  "issue tracker" is my memory and a small text file I have on my disk.

- Rich text capabilities. I'm generally not a big fan of rich text, but 
  Markdown IMO has found a pretty good balance. Also, it would allow 
  people to post images, which is nice since it is a GUI (IIRC, the 
  mailing list strips attachments).

- We reduce the noise on the Git list. Most people subscribed to the 
  list probably don't care about git-gui. So all git-gui related emails 
  are essentially noise for them. And while the volume has been 
  relatively low, it is not negligible.

Arguments against moving to GitHub:

- We lose the audience that we have on the mailing list. Every now and 
  then, people who are interested in Git, and not really in git-gui 
  chime in with help and suggestions. Those people are likely to not 
  want to follow the repo on GitHub, so we lose that insight. One 
  example that comes to my mind would be Denton Liu chiming in to help 
  with some commits that were missing from my tree.

- We depend on a non-open proprietary platform. While I personally don't 
  really care that much, some people might. But the truth remains that 
  it is a closed platform that I or any other contributor has no control 
  over. If GitHub decides to ban me or any other contributor tomorrow, 
  we can do nothing about it. This might sound far-fetched, but it has 
  happened recently [1]. This specific case did not touch public repos, 
  but this can change in the future. Something like this is less likely 
  to happen to the Git mailing list and email.

- People are restricted to the workflow prescribed by GitHub. With 
  email, there is a certain degree of flexibility/customizability with 
  how you sort your mail, how you configure your mail client, etc.

- Sub-threads are not supported. Each reply is only to the "main" 
  thread, and you can't reply to a reply (like you can do in email). It 
  is a minor thing, but worth a mention IMHO.

This list is not meant to be exhaustive in any way. The intention is to 
start listing out the benefits and losses so we can make an informed 
decision. Please feel free to add to this list (or remove from it).

[0] https://public-inbox.org/git/nycvar.QRO.7.76.6.1910061054470.46@tvgsbejvaqbjf.bet/
[1] https://www.theverge.com/2019/7/29/8934694/github-us-trade-sanctions-developers-restricted-crimea-cuba-iran-north-korea-syria

-- 
Regards,
Pratyush Yadav

Re: RFC: Moving git-gui development to GitHub

[Junio C Hamano]

Pratyush Yadav <me@yadavpratyush.com> writes:

> Arguments in favor of moving to GitHub:
>
> - Easier for new and one-off contributors.

It is uptimately up to you, the maintainer of the project, but
personally I feel "new and one-off" are way overvalued, after
considering if it serves the project and its users better to make it
easier for "new and one-off" contributors, or if it serves these
"new and one-off" contributors more than it benefits the project and
its users.

A quick rule of thumb I use is that it is worth spending my time on
training a new contributor (with hand-holding on workflows,
conventions, etc.) if it takes less than 3 times of effort compared
to doing the task myself (if I had infinite amount of time, that is)
for the first few topics the contributor works on.  You can usually
tell good ones after a few e-mail exchanges---their brilliance shine
through, even before they become familiar with particular conventions
of the project.

> - We reduce the noise on the Git list. Most people subscribed to the 
>   list probably don't care about git-gui. So all git-gui related emails 
>   are essentially noise for them. And while the volume has been 
>   relatively low, it is not negligible.

As long as the subject is marked clearly that the discussion is
about git-gui, it is easy to skip such an e-mail thread if a reader
is not interested in it.

This is related to the "we lose the audience" item on the other
list, but as a project that consumes the product of the git-core,
the needs of git-gui developers are of interest to git-core
developers.  If I am not mistaken, I think the recent topic about
logs/HEAD.lock by Dscho was to support what he's doing with git-gui,
and what the particular git-gui topic wanted from us happened to be
simple enough that we didn't have to dig too deeply the consumer
side in order to decide if the changes to git-core made sense, but
that may not always the case.

With a git-gui developer who is less experienced with git-core than
Dscho, it would be entirely plausible that the developer would try
to solve an issue on git-gui side with a lot more effort than
necessary because the developer is not familiar with git yet, and it
may turn out that it can be achieved easily with much less effort on
the git-gui side if we made a minimum and generic change on git-core
side.  The other way around is also possible; an inexperienced
git-gui developer may dream that miracles would solve an issue at
hand, and expect that git-core side may bend over backwards to
support an unreasonable one-off feature, which may never happen.

Re: RFC: Moving git-gui development to GitHub

[Birger Skogeng Pedersen]

Hi,

I would _love_ to see git-gui development moved to GitHub (or a
similar solution like Gitlab or Bitbucket). I have only submitted a
few patches, comments and new topics to this project. But I find the
use of mailing lists to be a _lot_ less efficient than using a
repository hosting service (like GitHub).

Advantages with GitHub
- Tidier than emails (very much so). Threads are neatly separated in a
single list, comments to threads are listed under their respective
thread. Threads can be linked.
- It's a lot easier to get a total overview of all issues. Especially
useful when you'd want to see open issues. Pratyush mentions he has a
text-file on his computer where he lists all the currently open
issues. We can't see this text-file. And even if we could, we'd still
have to navigate the mail archives to find the discussions and read
the emails one page at a time.
- It's a lot less hassle to submit patches (PRs), easier for everyone.

Disadvantages
- Git GUI contributors must have a GH account.
- All the data (threads, discussions, patches, etc) is not backed up
to such a large extent as it is when everyone has a copy of everything
in their email inboxes.


Best regards,
Birger

Re: RFC: Moving git-gui development to GitHub

[Denton Liu]

On Thu, Oct 24, 2019 at 09:37:08AM +0200, Birger Skogeng Pedersen wrote:
> - It's a lot easier to get a total overview of all issues. Especially
> useful when you'd want to see open issues. Pratyush mentions he has a
> text-file on his computer where he lists all the currently open
> issues. We can't see this text-file. And even if we could, we'd still
> have to navigate the mail archives to find the discussions and read
> the emails one page at a time.

I also had one of these text files laying around with my TODOs. Dscho
mentioned to me a while back that it might be a good idea to move
everything to GitGitGadget's issues. It's worked out pretty well for me
so far and I think a couple people have even picked up some work off of
there. It might be worth considering moving issues to either git-gui's
or GitGitGadget's repo.

If anything, it's no worse than the current situation since if GitHub
goes down for whatever reason, then Pratyush's list of issues is private
again. But as long as GitHub is up, then it'd be nice to have a public
list of issues for people to work on.

Re: RFC: Moving git-gui development to GitHub

[Pratyush Yadav]

On 24/10/19 10:16AM, Denton Liu wrote:
> On Thu, Oct 24, 2019 at 09:37:08AM +0200, Birger Skogeng Pedersen wrote:
> > - It's a lot easier to get a total overview of all issues. Especially
> > useful when you'd want to see open issues. Pratyush mentions he has a
> > text-file on his computer where he lists all the currently open
> > issues. We can't see this text-file. And even if we could, we'd still
> > have to navigate the mail archives to find the discussions and read
> > the emails one page at a time.
> 
> I also had one of these text files laying around with my TODOs. Dscho
> mentioned to me a while back that it might be a good idea to move
> everything to GitGitGadget's issues. It's worked out pretty well for me
> so far and I think a couple people have even picked up some work off of
> there. It might be worth considering moving issues to either git-gui's
> or GitGitGadget's repo.
> 
> If anything, it's no worse than the current situation since if GitHub
> goes down for whatever reason, then Pratyush's list of issues is private
> again. But as long as GitHub is up, then it'd be nice to have a public
> list of issues for people to work on.

That's a pretty good idea. Will do.

-- 
Regards,
Pratyush Yadav

Re: RFC: Moving git-gui development to GitHub

[Pratyush Yadav]

On 24/10/19 10:16AM, Denton Liu wrote:
> On Thu, Oct 24, 2019 at 09:37:08AM +0200, Birger Skogeng Pedersen wrote:
> > - It's a lot easier to get a total overview of all issues. Especially
> > useful when you'd want to see open issues. Pratyush mentions he has a
> > text-file on his computer where he lists all the currently open
> > issues. We can't see this text-file. And even if we could, we'd still
> > have to navigate the mail archives to find the discussions and read
> > the emails one page at a time.
> 
> I also had one of these text files laying around with my TODOs. Dscho
> mentioned to me a while back that it might be a good idea to move
> everything to GitGitGadget's issues. It's worked out pretty well for me
> so far and I think a couple people have even picked up some work off of
> there. It might be worth considering moving issues to either git-gui's
> or GitGitGadget's repo.
> 
> If anything, it's no worse than the current situation since if GitHub
> goes down for whatever reason, then Pratyush's list of issues is private
> again. But as long as GitHub is up, then it'd be nice to have a public
> list of issues for people to work on.

Update: the list can be now be found at [0].

[0] https://github.com/prati0100/git-gui/issues

-- 
Regards,
Pratyush Yadav

Re: RFC: Moving git-gui development to GitHub

[Birger Skogeng Pedersen]

Hi Pratyush,

On Thu, Oct 24, 2019 at 11:29 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
> Update: the list can be now be found at [0].
>
> [0] https://github.com/prati0100/git-gui/issues

Is that the full list? If so I'll add my issue [1] about automatically
selecting a staged file (avoid an empty diff) when focusing the commit
message widget.

[1] https://public-inbox.org/git/CAGr--=KMJmYtVaATFkOPcboAdkLvpZFbWAo4QAE0-uC6RL4Lqg@mail.gmail.com/

Birger

Re: RFC: Moving git-gui development to GitHub

[Pratyush Yadav]

On 25/10/19 07:33AM, Birger Skogeng Pedersen wrote:
> Hi Pratyush,
> 
> On Thu, Oct 24, 2019 at 11:29 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
> > Update: the list can be now be found at [0].
> >
> > [0] https://github.com/prati0100/git-gui/issues
> 
> Is that the full list? If so I'll add my issue [1] about automatically
> selecting a staged file (avoid an empty diff) when focusing the commit
> message widget.

Yes, it is the full list in the sense that it has all the items in my 
TODO list. But, it is not an "official" bug tracker. At least not until 
we switch to GitHub, if ever.

So, if you feel like it, go ahead and add your issue there. No problems. 
But you don't _have_ to.
 
> [1] https://public-inbox.org/git/CAGr--=KMJmYtVaATFkOPcboAdkLvpZFbWAo4QAE0-uC6RL4Lqg@mail.gmail.com/
> 
> Birger

-- 
Regards,
Pratyush Yadav

Re: RFC: Moving git-gui development to GitHub

[Elijah Newren]

On Thu, Oct 24, 2019 at 2:45 AM Pratyush Yadav <me@yadavpratyush.com> wrote:
>
> Hi everyone,
>
> I recently had some discussions with Dscho about whether it is a better
> idea to use GitHub for development instead of email [0]. His argument
> was that GitHub makes it easier for newcomers to contribute, since more
> people are familiar with GitHub compared to mailing lists. Also, it is
> somewhat difficult to set up an email-based workflow.

Interesting; I had been pondering asking the opposite question for
filter-repo: Even though filter-repo is tracked externally to git.git
(since we seem to want to move to a batteries-not-included model),
would it be okay to ask that filter-repo contributors send patches to
the git mailing list (possibly specially marked somehow)?

I'm debating between:
  - Ask contributors to send filter-repo patches to the git mailing
list (if okay).
  - Try out GerritHub (GitHub + Gerrit; see gerrithub.io) and maybe use it
  - Assume there won't be many contributions (wouldn't be surprising)
and put up with GitHub PRs

GitHub is great for ease of creating new repos, learning about other
developers, finding similar projects, creation of webhooks, etc.  But
it's *awful* for code review.  Gerrit is a lot better at code reviews
(though still has problems); so maybe dealing with both GitHub and
Gerrit would be reasonable.  (Reviewable also exists, and is kinda
decent, but I can't respect anything that doesn't offer reviewability
of commit messages.  And it makes me feel bad by making me want to
swat butterflies.)  Email is a horrible medium for sending/receiving
changes, but at least it gets the overall code review model right
(commit-messages-are-first-order-objects-that-can-be-reviewed,
review-individual-commits, merge-per-topic, cover-letter included,
range-diff for high-level comparison of different iterations,
reversing-commit-order-display-based-on-author-timestamps-is-NOT-forgivable,
Change-IDs-are-ugly, magic-refs-are-disgusting, etc.), something no
GUI tool (yet) does to my knowledge.


So...would anyone object if I asked filter-repo contributors to send
contributions via email to the git mailing list?

Thanks,
Elijah

Re: RFC: Moving git-gui development to GitHub

[Pratyush Yadav]

On 24/10/19 12:46PM, Elijah Newren wrote:
> On Thu, Oct 24, 2019 at 2:45 AM Pratyush Yadav <me@yadavpratyush.com> wrote:
> >
> > Hi everyone,
> >
> > I recently had some discussions with Dscho about whether it is a better
> > idea to use GitHub for development instead of email [0]. His argument
> > was that GitHub makes it easier for newcomers to contribute, since more
> > people are familiar with GitHub compared to mailing lists. Also, it is
> > somewhat difficult to set up an email-based workflow.
> 
> Interesting; I had been pondering asking the opposite question for
> filter-repo: Even though filter-repo is tracked externally to git.git
> (since we seem to want to move to a batteries-not-included model),
> would it be okay to ask that filter-repo contributors send patches to
> the git mailing list (possibly specially marked somehow)?

Marking them some way (like we do with git-gui) would probably be a good 
idea since it allows people to just skip over those threads if they're 
not interested. It also allows using email filters on those topics.
 
> I'm debating between:
>   - Ask contributors to send filter-repo patches to the git mailing
> list (if okay).
>   - Try out GerritHub (GitHub + Gerrit; see gerrithub.io) and maybe use it
>   - Assume there won't be many contributions (wouldn't be surprising)
> and put up with GitHub PRs
> 
> GitHub is great for ease of creating new repos, learning about other
> developers, finding similar projects, creation of webhooks, etc.  But
> it's *awful* for code review.  Gerrit is a lot better at code reviews
> (though still has problems); so maybe dealing with both GitHub and
> Gerrit would be reasonable.  (Reviewable also exists, and is kinda
> decent, but I can't respect anything that doesn't offer reviewability
> of commit messages.  And it makes me feel bad by making me want to
> swat butterflies.)  Email is a horrible medium for sending/receiving
> changes, but at least it gets the overall code review model right
> (commit-messages-are-first-order-objects-that-can-be-reviewed,
> review-individual-commits, merge-per-topic, cover-letter included,
> range-diff for high-level comparison of different iterations,
> reversing-commit-order-display-based-on-author-timestamps-is-NOT-forgivable,
> Change-IDs-are-ugly, magic-refs-are-disgusting, etc.), something no
> GUI tool (yet) does to my knowledge.

Thanks for your perspective. I have never really used GitHub for 
anything more than one off contributions, and so never really ended up 
using their review and merge tools too much. In fact, most of the open 
source projects I have been interested in used mailing lists, which is 
why I questioned myself if I'm biased towards such a workflow.
 
> So...would anyone object if I asked filter-repo contributors to send
> contributions via email to the git mailing list?

-- 
Regards,
Pratyush Yadav

Re: RFC: Moving git-gui development to GitHub

[Jakub Narebski]

Elijah Newren <newren@gmail.com> writes:
> On Thu, Oct 24, 2019 at 2:45 AM Pratyush Yadav <me@yadavpratyush.com> wrote:
>>
>> I recently had some discussions with Dscho about whether it is a better
>> idea to use GitHub for development instead of email [0]. His argument
>> was that GitHub makes it easier for newcomers to contribute, since more
>> people are familiar with GitHub compared to mailing lists. Also, it is
>> somewhat difficult to set up an email-based workflow.
[...]
> GitHub is great for ease of creating new repos, learning about other
> developers, finding similar projects, creation of webhooks, etc.  But
> it's *awful* for code review.  Gerrit is a lot better at code reviews
> (though still has problems); so maybe dealing with both GitHub and
> Gerrit would be reasonable.
[...]
> Email is a horrible medium for sending/receiving
> changes, but at least it gets the overall code review model right
> (commit-messages-are-first-order-objects-that-can-be-reviewed,
> review-individual-commits, merge-per-topic, cover-letter included,
> range-diff for high-level comparison of different iterations,
> reversing-commit-order-display-based-on-author-timestamps-is-NOT-forgivable,
> Change-IDs-are-ugly, magic-refs-are-disgusting, etc.), something no
> GUI tool (yet) does to my knowledge.

I agree with that.  You need then to decide whether it is better to have
it easier for beginners to contribute, or is it better to have it easier
to review code.  What are the pain points?


Another source worth looking into is "Patches carved into stone tablets,
why the Linux kernel developers rely on plain text email instead of
using “modern” development tools." presentation by Greg KH from
2016[1][2].  But remember that git-gui is not Linux kernel; what works
for one might not work for the other.

It is unfortunate that we have no tools described in "Patches carved
into developer sigchains"[3] wishfull blog post by Konstantin Ryabitsev...

[1]: https://kernel-recipes.org/en/2016/talks/patches-carved-into-stone-tablets/
[2]: https://www.slideshare.net/ennael/kernel-recipes-2016-patches-carved-into-stone-tablets
[3]: https://people.kernel.org/monsieuricon/patches-carved-into-developer-sigchains

Regards,
--
Jakub Narębski

Re: RFC: Moving git-gui development to GitHub

[Konstantin Ryabitsev]

On Sat, Oct 26, 2019 at 08:25:40PM +0200, Jakub Narebski wrote:
> It is unfortunate that we have no tools described in "Patches carved
> into developer sigchains"[3] wishfull blog post by Konstantin Ryabitsev...

We are working to get us there. It's going to be a long process and we
are choosing the evolutionary approach as opposed to staging a
revolution. :)

-K

Re: RFC: Moving git-gui development to GitHub

[Elijah Newren]

On Sat, Oct 26, 2019 at 11:25 AM Jakub Narebski <jnareb@gmail.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
> > On Thu, Oct 24, 2019 at 2:45 AM Pratyush Yadav <me@yadavpratyush.com> wrote:
> >>
> >> I recently had some discussions with Dscho about whether it is a better
> >> idea to use GitHub for development instead of email [0]. His argument
> >> was that GitHub makes it easier for newcomers to contribute, since more
> >> people are familiar with GitHub compared to mailing lists. Also, it is
> >> somewhat difficult to set up an email-based workflow.
> [...]
> > GitHub is great for ease of creating new repos, learning about other
> > developers, finding similar projects, creation of webhooks, etc.  But
> > it's *awful* for code review.  Gerrit is a lot better at code reviews
> > (though still has problems); so maybe dealing with both GitHub and
> > Gerrit would be reasonable.
> [...]
> > Email is a horrible medium for sending/receiving
> > changes, but at least it gets the overall code review model right
> > (commit-messages-are-first-order-objects-that-can-be-reviewed,
> > review-individual-commits, merge-per-topic, cover-letter included,
> > range-diff for high-level comparison of different iterations,
> > reversing-commit-order-display-based-on-author-timestamps-is-NOT-forgivable,
> > Change-IDs-are-ugly, magic-refs-are-disgusting, etc.), something no
> > GUI tool (yet) does to my knowledge.
>
> I agree with that.  You need then to decide whether it is better to have
> it easier for beginners to contribute, or is it better to have it easier
> to review code.  What are the pain points?

I don't think that's the right comparison to make.  The problems with
GitHub code review aren't solely ease-of-use issues, they are more a
quality-of-code issues.

Projects which switch to GitHub tend to have overall commit quality go
down IMO, because the system (a) makes it nearly impossible to review
commit messages, so people eventually degrade to writing really bad
ones, (b) makes it nearly impossible to review a rebased set of
changes except redoing the entire review from square one, so people
don't rebase, (c) punishes both users and reviewers who want to work
with a rebased patch series by displaying the series out of order --
even for a completely linear history (it resorts based on author
timestamp, not even committer timestamp), and (d) punishes
reviewers/users when they attempt to review individual commits by
making it harder to see and follow these comments (though it has
gotten much better on this front).  There are combination effects too.
People to write really bad commit messages for all the additional
"fixups" they have.  People notice that commits don't bisect nicely,
and instead of understanding that the broken code review system they
wrote was the problem, they instead offer a new "squash merge" option,
thus destroying all the carefully separated commits that help people
understand the individual steps toward the new feature and making it
impossible for anyone in the future to review it incrementally.  You
may say it's a workflow choice for some people to just squash all
their stuff together at their option, which would be fine, but the
problem is most developers don't take the time to think, and someone
in charge of the project notices that they keep getting un-bisectable
meaningless commits unless they force *everyone* in the project to use
squash merging.  Now they are punishing me for creating clean separate
commits and forcing them all to be squashed -- all as an ugly
workaround to the basic tool being *broken*.  You can work around this
by making a long sequence of PRs, one per what you intend to be a
commit, and try to track the hierarchy -- something that GitHub
certainly doesn't make easy.  And then each PR becomes a trivial small
change, and you are back to merging individual commits, and writing
your own tools to manage a hierarchy of PRs...and reviewers hate you
if you do that because it's extremely onerous on them.

GitHub PRs aren't just hard to use, they literally degrade the quality
of the code for people who have to use it.  I've seen it happen with
many projects.

(At $DAYJOB, they have hundreds of repos and have at times used SVN
then gitolite then gerrit then (Atlassian) stash then github (with
other review tools like sourcegraph and reviewable tried and a few
others read up on), with most of those existing simultaneously --
though we eventually pruned it down to just Gerrit and GitHub, with
some projects in one and some in the other system.  I've seen
migrations between various combinations of these tools (though SVN and
gitolite were nearly phased out by the time I joined), and seen
results of how the tools caused differences in behavior.  And yes, I
know that GitHub's popularity means many have copied GitHub's PR
model, from sourcegraph who were basically identical, to Stash which
is similar but handled rebases better, and I think GitLab looks
similar though I haven't used it that much (yet).  Reviewable handled
rebases a lot better, but still used the utterly broken model of not
allowing commit messages to be reviewed -- and glitterbombed the
interface with butterflies.)

> Another source worth looking into is "Patches carved into stone tablets,
> why the Linux kernel developers rely on plain text email instead of
> using “modern” development tools." presentation by Greg KH from
> 2016[1][2].  But remember that git-gui is not Linux kernel; what works
> for one might not work for the other.
>
> It is unfortunate that we have no tools described in "Patches carved
> into developer sigchains"[3] wishfull blog post by Konstantin Ryabitsev...

Interesting links; thanks for providing them.  And yes, I agree that
git-gui won't necessarily have the same tradeoffs as the Linux kernel.
But I do tend to be noisy here hoping I can spark someone with a
modicum of talent for front-end development (I have neither the talent
nor the inclination for web development) to write a sane webby code
review system.  Much of GitHub is awesome.  And it has some apparently
really nice thing for web devs, which seems to be part of why so many
people use it.  It's just so terribly awful at code reviews.  But
systems like Reviewable and GerritHub build on top of GitHub, so maybe
there is hope someone can build just the review part and make it
awesome.

> [1]: https://kernel-recipes.org/en/2016/talks/patches-carved-into-stone-tablets/
> [2]: https://www.slideshare.net/ennael/kernel-recipes-2016-patches-carved-into-stone-tablets
> [3]: https://people.kernel.org/monsieuricon/patches-carved-into-developer-sigchains
>
> Regards,
> --
> Jakub Narębski

Re: RFC: Moving git-gui development to GitHub

[Birger Skogeng Pedersen]

Hei Elijah,

On Wed, Oct 30, 2019 at 7:21 AM Elijah Newren <newren@gmail.com> wrote:
> Projects which switch to GitHub tend to have overall commit quality go
> down IMO, because the system (a) makes it nearly impossible to review
> commit messages, so people eventually degrade to writing really bad
> ones,
What do you mean here, exactly? In what way is it "nearly impossible"
to review commit messages in GH?

br
Birger

Re: RFC: Moving git-gui development to GitHub

[Elijah Newren]

On Wed, Nov 20, 2019 at 4:20 AM Birger Skogeng Pedersen
<birger.sp@gmail.com> wrote:
>
> Hei Elijah,
>
> On Wed, Oct 30, 2019 at 7:21 AM Elijah Newren <newren@gmail.com> wrote:
> > Projects which switch to GitHub tend to have overall commit quality go
> > down IMO, because the system (a) makes it nearly impossible to review
> > commit messages, so people eventually degrade to writing really bad
> > ones,
> What do you mean here, exactly? In what way is it "nearly impossible"
> to review commit messages in GH?

My lengthy rant wasn't good enough for you?  ;-)  Well, I'll try even
harder to bore everyone to death, then and extend my rant a bit...


Reviewing is the process of providing feedback on proposed changes.
Code review tools and mechanisms typically provide ways to (a) see
proposed changes in isolation and (b) comment on individual lines and
preserve context (with the goal of later merging a group of commits
that implement something useful).

git-format-patch and git-send-email combined with usage of email
clients that know how to quote previous emails and let you respond
inline are a natural way of achieving both (a) and (b).

GUI tools can, of course, also achieve something similar by showing
proposed changes and allowing commenting on individual lines in
context.  GitHub fails pretty hard on both counts, particularly for
commit messages.  It guides people to an overall diff rather than to
the diffs inside individual commits and completely omits all commit
messages when you do so.  It does provide a way to access individual
commits and their diffs, though it makes it somewhat unnatural.  (It
at least isn't as awful as it used to be in years past, when any
comments on individual commits were completely lost and separated from
the PR.)  And even if you do "go against the grain" to comment on
individual commits, there is no provided mechanism for commenting on
the commit message itself.  Instead it has to be given as a general
comment on the overall set of changes, which then loses the context of
what you are commenting on unless you re-include and quote it
yourself.  That usually doesn't happen, so when you comment on four
commit messages in a review, you have four separate global comments
and someone attempting to respond to them doesn't get to see the
commit messages next to them, resulting in confusion.  Even if you do
re-include and quote the commit message bits you are commenting on,
the resulting comment isn't in any way tied to the commit in question
in the UI.

So people who use GitHub for code review just don't bother.   They
write non-isolated commits and far from rarely use awful commit
messages.  Then they merge this abomination of history, or possibly
even worse, they squash merge it all to make it impossible for any
future readers to be able to dissect.

Yeah, yeah, small features so that the review is smaller and easier.
That is important, yes, but it still conflates two things and thus
ruins reviews.  Each PR should implement something useful.  Commits
should be designed both for current and future reviewers to see a
clear path towards how that useful thing was implemented.  Sometimes
one commit is enough, but conflating the two necessarily either means
sometimes creating one-commit PRs that don't actually implement
anything useful, or a cognitive overload for code reviewers.  GitHub
simultaneously encourages bad behavior (bad commit messages since they
are designed to not be reviewable, non-isolated commits, fixup commits
that are never properly squashed, etc.) and penalizes good behavior
(folks who try to clean up their sequence of commits are met with
problems ranging from GitHub screwing up the topological ordering of a
linear commit history, to poor ability to see incremental changes
whenever rebasing happens, to reckless squash merging of all their
careful work into a single commit as something close to an act of war
against any future readers who want to dig into why certain changes
were made).  Yes, GitHub has gotten much better at code reviews; it's
merely abysmally awful these days as opposed to a complete joke as it
was in years past.  But it's still so bad that I have seen people try
to solve this by having a sequence of PRs per (small) feature they
want to implement, even though GitHub provides no way to denote
dependencies or ordering between PRs.

You may think I've gone off on a bunch of tangents, but fundamentally
I believe that almost all of these other problems predominantly arise
as secondary and tertiary effects of not understanding that commit
messages should be a first class citizen of code review.

Sure, you can claim all you want that it is entirely possible to
review commit messages within the GitHub UI and it's just extremely
inconvenient, yadda, yadda, but the truth of the matter is that people
everywhere struggle to even do code reviews at all, and when they do
they all too often turn into rubberstamp exercises or don't delve
deeply enough.  In that context, I believe my "nearly impossible"
wording is entirely warranted.  Using a tool that simultaneously
encourages bad behavior and penalizes good behavior will not so
surprisingly yield bad behavior.  GitHub PRs are such a tool, IMO.

(To be fair, I'll note that GitHub has awesome code browsing, really
easy setup and administration of new repositories and organizations,
simple and reasonable and thus pretty nice code search, etc., etc.
I'm not saying GitHub is a bad tool, I actually think most of it is a
very excellent tool; I am just claiming that the PR section of it is
very bad.)


Elijah

Pain points in PRs [was: Re: RFC: Moving git-gui development to GitHub]

[SZEDER Gábor]

On Wed, Nov 20, 2019 at 09:13:04AM -0800, Elijah Newren wrote:
> > On Wed, Oct 30, 2019 at 7:21 AM Elijah Newren <newren@gmail.com> wrote:
> > > Projects which switch to GitHub tend to have overall commit quality go
> > > down IMO, because the system (a) makes it nearly impossible to review
> > > commit messages, so people eventually degrade to writing really bad
> > > ones,
> > What do you mean here, exactly? In what way is it "nearly impossible"
> > to review commit messages in GH?
> 
> My lengthy rant wasn't good enough for you?  ;-)  Well, I'll try even
> harder to bore everyone to death, then and extend my rant a bit...

Thank you very much for taking the time and effort to write it up.

It summarized some of my main gripes with PR-based collaboration on
GitHub with such clarity that I would never been able to achive.

(The recent "Pain points in Git's patch flow" thread reminded me that
I saved this message and even marked it as important ages ago... but
haven't gotten around to read it until now.

  https://public-inbox.org/git/YHaIBvl6Mf7ztJB3@google.com/T/
)

> Reviewing is the process of providing feedback on proposed changes.
> Code review tools and mechanisms typically provide ways to (a) see
> proposed changes in isolation and (b) comment on individual lines and
> preserve context (with the goal of later merging a group of commits
> that implement something useful).
> 
> git-format-patch and git-send-email combined with usage of email
> clients that know how to quote previous emails and let you respond
> inline are a natural way of achieving both (a) and (b).
> 
> GUI tools can, of course, also achieve something similar by showing
> proposed changes and allowing commenting on individual lines in
> context.  GitHub fails pretty hard on both counts, particularly for
> commit messages.  It guides people to an overall diff rather than to
> the diffs inside individual commits and completely omits all commit
> messages when you do so.  It does provide a way to access individual
> commits and their diffs, though it makes it somewhat unnatural.  (It
> at least isn't as awful as it used to be in years past, when any
> comments on individual commits were completely lost and separated from
> the PR.)  And even if you do "go against the grain" to comment on
> individual commits, there is no provided mechanism for commenting on
> the commit message itself.  Instead it has to be given as a general
> comment on the overall set of changes, which then loses the context of
> what you are commenting on unless you re-include and quote it
> yourself.  That usually doesn't happen, so when you comment on four
> commit messages in a review, you have four separate global comments
> and someone attempting to respond to them doesn't get to see the
> commit messages next to them, resulting in confusion.  Even if you do
> re-include and quote the commit message bits you are commenting on,
> the resulting comment isn't in any way tied to the commit in question
> in the UI.
> 
> So people who use GitHub for code review just don't bother.   They
> write non-isolated commits and far from rarely use awful commit
> messages.  Then they merge this abomination of history, or possibly
> even worse, they squash merge it all to make it impossible for any
> future readers to be able to dissect.
> 
> Yeah, yeah, small features so that the review is smaller and easier.
> That is important, yes, but it still conflates two things and thus
> ruins reviews.  Each PR should implement something useful.  Commits
> should be designed both for current and future reviewers to see a
> clear path towards how that useful thing was implemented.  Sometimes
> one commit is enough, but conflating the two necessarily either means
> sometimes creating one-commit PRs that don't actually implement
> anything useful, or a cognitive overload for code reviewers.  GitHub
> simultaneously encourages bad behavior (bad commit messages since they
> are designed to not be reviewable, non-isolated commits, fixup commits
> that are never properly squashed, etc.) and penalizes good behavior
> (folks who try to clean up their sequence of commits are met with
> problems ranging from GitHub screwing up the topological ordering of a
> linear commit history, to poor ability to see incremental changes
> whenever rebasing happens, to reckless squash merging of all their
> careful work into a single commit as something close to an act of war
> against any future readers who want to dig into why certain changes
> were made).  Yes, GitHub has gotten much better at code reviews; it's
> merely abysmally awful these days as opposed to a complete joke as it
> was in years past.  But it's still so bad that I have seen people try
> to solve this by having a sequence of PRs per (small) feature they
> want to implement, even though GitHub provides no way to denote
> dependencies or ordering between PRs.
> 
> You may think I've gone off on a bunch of tangents, but fundamentally
> I believe that almost all of these other problems predominantly arise
> as secondary and tertiary effects of not understanding that commit
> messages should be a first class citizen of code review.
> 
> Sure, you can claim all you want that it is entirely possible to
> review commit messages within the GitHub UI and it's just extremely
> inconvenient, yadda, yadda, but the truth of the matter is that people
> everywhere struggle to even do code reviews at all, and when they do
> they all too often turn into rubberstamp exercises or don't delve
> deeply enough.  In that context, I believe my "nearly impossible"
> wording is entirely warranted.  Using a tool that simultaneously
> encourages bad behavior and penalizes good behavior will not so
> surprisingly yield bad behavior.  GitHub PRs are such a tool, IMO.
> 
> (To be fair, I'll note that GitHub has awesome code browsing, really
> easy setup and administration of new repositories and organizations,
> simple and reasonable and thus pretty nice code search, etc., etc.
> I'm not saying GitHub is a bad tool, I actually think most of it is a
> very excellent tool; I am just claiming that the PR section of it is
> very bad.)
> 
> 
> Elijah

Re: Pain points in PRs [was: Re: RFC: Moving git-gui development to GitHub]

[Junio C Hamano]

SZEDER Gábor <szeder.dev@gmail.com> writes:

> On Wed, Nov 20, 2019 at 09:13:04AM -0800, Elijah Newren wrote:
>> > On Wed, Oct 30, 2019 at 7:21 AM Elijah Newren <newren@gmail.com> wrote:
>> > > Projects which switch to GitHub tend to have overall commit quality go
>> > > down IMO, because the system (a) makes it nearly impossible to review
>> > > commit messages, so people eventually degrade to writing really bad
>> > > ones,
>> > What do you mean here, exactly? In what way is it "nearly impossible"
>> > to review commit messages in GH?
>> 
>> My lengthy rant wasn't good enough for you?  ;-)  Well, I'll try even
>> harder to bore everyone to death, then and extend my rant a bit...
>
> Thank you very much for taking the time and effort to write it up.
>
> It summarized some of my main gripes with PR-based collaboration on
> GitHub with such clarity that I would never been able to achive.
>
> (The recent "Pain points in Git's patch flow" thread reminded me that
> I saved this message and even marked it as important ages ago... but
> haven't gotten around to read it until now.
>
>   https://public-inbox.org/git/YHaIBvl6Mf7ztJB3@google.com/T/
> )

Interesting.

I recently had a similar experience with Gerrit, where a patch I
have seen quite a few times on Gerrit at $WORK had an embarrassing
syntactic issues I did not discover until it hit the public mailing
list.  It may be different from reviewer to reviewer, but at least
to me, e-mailed workflow forces me to apply the patch to my tree
before I can say anything non-trivially intelligent about it and
once applied to the tree, it actually let's me play with the code
(like, say, asking the compiler to give its opinion on it).

The experience I had with Gerrit at $WORK gave me side-to-side diff
with context with arbitrary on-demand width, even with per-word
differences highlighted, and it may be wonderful that I can get all
of these _without_ having to apply the patch myself, but what it
gave me stopped there.  There are a lot more things that need to
happen beyond looking at what changed in the context of the files
during a review, from grepping in the tree for functions and
variables used in the patch to see their uses in other parts of the
system that the patch does not touch, to make various trial merges
to different topics that are in flight, and Gerrit didn't help me an
iota, but still gave me a (false) impression that I _did_ review the
patch fully, when I only have scraped its surface, and the worst
part of the story was that the UI feld so nice that I didn't even
realize that I was doing a lot more shoddy job in reviewing than
what I usually do to e-mailed patches.


>> Reviewing is the process of providing feedback on proposed changes.
>> Code review tools and mechanisms typically provide ways to (a) see
>> proposed changes in isolation and (b) comment on individual lines and
>> preserve context (with the goal of later merging a group of commits
>> that implement something useful).
>> 
>> git-format-patch and git-send-email combined with usage of email
>> clients that know how to quote previous emails and let you respond
>> inline are a natural way of achieving both (a) and (b).
>> 
>> GUI tools can, of course, also achieve something similar by showing
>> proposed changes and allowing commenting on individual lines in
>> context.  GitHub fails pretty hard on both counts, particularly for
>> commit messages.  It guides people to an overall diff rather than to
>> the diffs inside individual commits and completely omits all commit
>> messages when you do so.  It does provide a way to access individual
>> commits and their diffs, though it makes it somewhat unnatural.  (It
>> at least isn't as awful as it used to be in years past, when any
>> comments on individual commits were completely lost and separated from
>> the PR.)  And even if you do "go against the grain" to comment on
>> individual commits, there is no provided mechanism for commenting on
>> the commit message itself.  Instead it has to be given as a general
>> comment on the overall set of changes, which then loses the context of
>> what you are commenting on unless you re-include and quote it
>> yourself.  That usually doesn't happen, so when you comment on four
>> commit messages in a review, you have four separate global comments
>> and someone attempting to respond to them doesn't get to see the
>> commit messages next to them, resulting in confusion.  Even if you do
>> re-include and quote the commit message bits you are commenting on,
>> the resulting comment isn't in any way tied to the commit in question
>> in the UI.
>> 
>> So people who use GitHub for code review just don't bother.   They
>> write non-isolated commits and far from rarely use awful commit
>> messages.  Then they merge this abomination of history, or possibly
>> even worse, they squash merge it all to make it impossible for any
>> future readers to be able to dissect.
>> 
>> Yeah, yeah, small features so that the review is smaller and easier.
>> That is important, yes, but it still conflates two things and thus
>> ruins reviews.  Each PR should implement something useful.  Commits
>> should be designed both for current and future reviewers to see a
>> clear path towards how that useful thing was implemented.  Sometimes
>> one commit is enough, but conflating the two necessarily either means
>> sometimes creating one-commit PRs that don't actually implement
>> anything useful, or a cognitive overload for code reviewers.  GitHub
>> simultaneously encourages bad behavior (bad commit messages since they
>> are designed to not be reviewable, non-isolated commits, fixup commits
>> that are never properly squashed, etc.) and penalizes good behavior
>> (folks who try to clean up their sequence of commits are met with
>> problems ranging from GitHub screwing up the topological ordering of a
>> linear commit history, to poor ability to see incremental changes
>> whenever rebasing happens, to reckless squash merging of all their
>> careful work into a single commit as something close to an act of war
>> against any future readers who want to dig into why certain changes
>> were made).  Yes, GitHub has gotten much better at code reviews; it's
>> merely abysmally awful these days as opposed to a complete joke as it
>> was in years past.  But it's still so bad that I have seen people try
>> to solve this by having a sequence of PRs per (small) feature they
>> want to implement, even though GitHub provides no way to denote
>> dependencies or ordering between PRs.
>> 
>> You may think I've gone off on a bunch of tangents, but fundamentally
>> I believe that almost all of these other problems predominantly arise
>> as secondary and tertiary effects of not understanding that commit
>> messages should be a first class citizen of code review.
>> 
>> Sure, you can claim all you want that it is entirely possible to
>> review commit messages within the GitHub UI and it's just extremely
>> inconvenient, yadda, yadda, but the truth of the matter is that people
>> everywhere struggle to even do code reviews at all, and when they do
>> they all too often turn into rubberstamp exercises or don't delve
>> deeply enough.  In that context, I believe my "nearly impossible"
>> wording is entirely warranted.  Using a tool that simultaneously
>> encourages bad behavior and penalizes good behavior will not so
>> surprisingly yield bad behavior.  GitHub PRs are such a tool, IMO.
>> 
>> (To be fair, I'll note that GitHub has awesome code browsing, really
>> easy setup and administration of new repositories and organizations,
>> simple and reasonable and thus pretty nice code search, etc., etc.
>> I'm not saying GitHub is a bad tool, I actually think most of it is a
>> very excellent tool; I am just claiming that the PR section of it is
>> very bad.)
>> 
>> 
>> Elijah

Re: Pain points in PRs [was: Re: RFC: Moving git-gui development to GitHub]

[Son Luong Ngoc]

Hi Junio,

On Mon, Apr 19, 2021 at 02:52:16PM -0700, Junio C Hamano wrote:
> 
> Interesting.
> 
> I recently had a similar experience with Gerrit, where a patch I
> have seen quite a few times on Gerrit at $WORK had an embarrassing
> syntactic issues I did not discover until it hit the public mailing
> list.  It may be different from reviewer to reviewer, but at least
> to me, e-mailed workflow forces me to apply the patch to my tree
> before I can say anything non-trivially intelligent about it and
> once applied to the tree, it actually let's me play with the code
> (like, say, asking the compiler to give its opinion on it).
> 

I think this is very much the point of having a good CI pipeline:
  - Apply patches into tree
  - Compile
  - Run relevant tests

I'm not sure about Github PR, but Gitlab's MR workflow also provide a
merge queue implementation(Merge Train) coupled with CI to ensure the
merge result is accurately verified against tests.

What might be missing from (most) CI services is a bisect pipeline that
help us identify culprit commit that broke the tests, but that could be
engineered.

> The experience I had with Gerrit at $WORK gave me side-to-side diff
> with context with arbitrary on-demand width, even with per-word
> differences highlighted, and it may be wonderful that I can get all
> of these _without_ having to apply the patch myself, but what it
> gave me stopped there.  There are a lot more things that need to
> happen beyond looking at what changed in the context of the files
> during a review, from grepping in the tree for functions and
> variables used in the patch to see their uses in other parts of the
> system that the patch does not touch, to make various trial merges
> to different topics that are in flight, and Gerrit didn't help me an
> iota, but still gave me a (false) impression that I _did_ review the
> patch fully, when I only have scraped its surface, and the worst
> part of the story was that the UI feld so nice that I didn't even
> realize that I was doing a lot more shoddy job in reviewing than
> what I usually do to e-mailed patches.
> 

Yes, having context beyond the diff is very important for Code Review.
This is why I strongly recommend SourceGraph usages to folks I know.

  > https://sourcegraph.com/github.com/git/git/-/blob/builtin/repack.c#L61:13
  > https://sourcegraph.com/github.com/git/git/-/commit/9218c6a40c37023a1f434222d501218cf8157857#diff-01ec5e99d04fb7ba9753f219ab638469R64

(I have no affiliation with SourceGraph, just really enjoy their product)

A mordern codesearch service like sourcegraph could help decorate diff
with relevant code intelligent like finding references, definitions and
assist with the Code Review process.

Afaik, sourcegraph has been building more integrations with Github and
Gitlab, not too sure about Gerrit (but Im sure it's not far reach given
their GraphQL API).

So I guess mordern toolings are available for these usecases, but
fragmented and subjective to personal workflow.

Regards,
Son Luong.

Re: Pain points in PRs [was: Re: RFC: Moving git-gui development to GitHub]

[Junio C Hamano]

Son Luong Ngoc <sluongng@gmail.com> writes:

> On Mon, Apr 19, 2021 at 02:52:16PM -0700, Junio C Hamano wrote:
>> 
>> Interesting.
>> 
>> I recently had a similar experience with Gerrit, where a patch I
>> have seen quite a few times on Gerrit at $WORK had an embarrassing
>> syntactic issues I did not discover until it hit the public mailing
>> list.  It may be different from reviewer to reviewer, but at least
>> to me, e-mailed workflow forces me to apply the patch to my tree
>> before I can say anything non-trivially intelligent about it and
>> once applied to the tree, it actually let's me play with the code
>> (like, say, asking the compiler to give its opinion on it).
>> 
>
> I think this is very much the point of having a good CI pipeline:
>   - Apply patches into tree
>   - Compile
>   - Run relevant tests

It is true that CI can spot -Wdecl-after-stmt, but CI only covers
just one part of what is needed while I do my reviews.  It would
also be doable with web interface to look at all the places that
functions modified by the patch are referred to, and to check if the
change makes sense in the context of the entire tree.  It would also
be doable with web interface to looking at the evolution of the code
being changed.  There are some things, like building and using for
everyday life, running the built binary under debuggers, etc., that
may be harder to do with web interface, but I am sure many things
would become doable given enough time and effort.  However.

> Yes, having context beyond the diff is very important for Code Review.
> This is why I strongly recommend SourceGraph usages to folks I know.
> ...
> So I guess mordern toolings are available for these usecases, but
> fragmented and subjective to personal workflow.

My point in the message you are responding to was that I can do all
what is necessary locally, with my favorite toolset, once I apply a
patch to my tree.  The only thing that Gerrit allowed me to skip in
my recent adventure was to download the patch and apply to a newly
created topic branch locally to my tree, before I can start doing
some of the things (e.g. "look at the patch, examine with larger
context as needed", "grep for the symbols at the same revision in
paths that are not touched by the patch") that was needed to review.
And while I know I shouldn't blame the tool for this, but it did
mislead me to false sense of "I've reviewed this change well enough",
when I haven't.

By the way, I've been playing with "b4 am" and it's been a pleasant
experience so far.

Thanks.

RE: Pain points in PRs [was: Re: RFC: Moving git-gui development to GitHub]

[Felipe Contreras]

SZEDER Gábor wrote:
> On Wed, Nov 20, 2019 at 09:13:04AM -0800, Elijah Newren wrote:
> > > On Wed, Oct 30, 2019 at 7:21 AM Elijah Newren <newren@gmail.com> wrote:
> > > > Projects which switch to GitHub tend to have overall commit quality go
> > > > down IMO, because the system (a) makes it nearly impossible to review
> > > > commit messages, so people eventually degrade to writing really bad
> > > > ones,
> > > What do you mean here, exactly? In what way is it "nearly impossible"
> > > to review commit messages in GH?
> > 
> > My lengthy rant wasn't good enough for you?  ;-)  Well, I'll try even
> > harder to bore everyone to death, then and extend my rant a bit...
> 
> Thank you very much for taking the time and effort to write it up.
> 
> It summarized some of my main gripes with PR-based collaboration on
> GitHub with such clarity that I would never been able to achive.
> 
> (The recent "Pain points in Git's patch flow" thread reminded me that
> I saved this message and even marked it as important ages ago... but
> haven't gotten around to read it until now.
> 
>   https://public-inbox.org/git/YHaIBvl6Mf7ztJB3@google.com/T/
> )

People in general follow the path of least resistance; if you make X
harder, people will spend more effort doing X, but they will do less of
it.

People have been using email for decades, and there's all kinds of tools
for dealing with it (I for example am using a free provider [Gmail], a
tool to download part of my mails [mbsync], another tool to index it
[notmuch], yet another tool to read it [notmuch-vim], yet another tool
to write a response [vim], and I will be using another to send it
[msmtp]). Or I cound simply use the Gmail app on my mobile phone.

Email is extremely convenient.

Since it's easy to reply, people do reply, often.

Which is why it's not rare at all that a patch series becomes a
discussion thread, which are easy to deal with through email.

GitHub adds a layer of inconvenience, so people tend to avoid big
discussions in a GitHub pull request. It's just not fun.

I also did a blog post explaining why email is just superior [1].

Two other points that were not mentioned that make email superior is
that 1) you can easily cross-post, simply CC another project and that
discussion spreads 2) it scales for projects that don't use GitHub; you
don't need an account anywhere, and usually you don't need to subscribe
to post in the mailing list.

It's no coincidence that the most successful project in history (Linux)
uses email to deal with contributions: nothing else comes even close.

Cheers.

[1] https://felipec.wordpress.com/2010/01/19/why-bugzilla-sucks-for-handling-patches/

-- 
Felipe Contreras