Re: FYI: Status of gdb usage of gerrit

[Carlos O'Donell <carlos at redhat dot com>]

On 11/27/19 3:46 PM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> If we don't discuss this then we can't learn what areas gerrit was bad 
>> at and apply the same critera of evaluation for the next tool.
> 
> Okay, here's my list.

Thank you for this, the list is *great* and I enjoyed reading it.

> # UI issues
> 
> * The auto-complete list selector cannot select elements that are a
>   substring of many elements.  For example, you cannot select the
>   master branch in a branch selector.
> 
> * The terminoly is confusing initially.  Gerrit calls patch
>   submissions “reviews”.  For example, when exiting WIP status, the
>   button is labeled “Start a review”, when I would have expected
>   something like “Submit for review”.
> 
> * A comment needing action is in a slightly darker yellow than a
>   comment that does not.  The difference is not very prominent even
>   with a high-contrast screen.
> 
> * It is apparently not possible to see a list of reviews I have
>   submitted which have unresolved comments.  This means I have to
>   trust the +1s and +2s to be accurate.
> 
> * Overall, there is very little guidance about what to do next based
>   on what the tool knows about you.  Maybe it's possible to get a real
>   dashboard by configuring queries properly.  But the defaults do not
>   appear very useful to me.
> 
> * The comment thread view at the top-level change page shows review
>   comments, but there's no apparent link to view them in context (to
>   review the previous discussion).  The link that's there doesn't
>   reliably lead to the discussion.  And for a comment on an entire
>   patchset, there's no link to that patchset.
> 
> * There merge conflict notification is helpful, but it's not very
>   smart.  Even if the conflict is concentrated to the beginning of a
>   series (e.g. the first patch is the only one updating NEWS, and the
>   conflict is in that file), all of the series is flagged with Merged
>   Conflict.
> 
> * It's not clear when patches are marked as Rebase Needed.  Clearly
>   trivial rebases are exempted.
> 
> * It's unclear what it means when a patch to a file (in a patchset)
>   has been reviewed.  The recorded state does not seem to matter to
>   pretty much anything at all.  I haven't seen a warning when
>   submitting a +2 with some parts unreviewed.
> 
> # Email notifications
> 
> * Comments on other comments quote the diff hunk, but not the previous
>   comments.  Often you have to try to find them in the web tool to
>   make sense of them.
> 
> * No email notification for new patches transitioning out of WIP
>   status.  (Already mentioned elsewhere.)  This makes %wip rather
>   useless.
> 
> * There were problems with email signup.  Not sure if they have been
>   resolved.  It's like something specific to our instance.
> 
> * The email notifications are somewhat chatty.  Part of that is
>   probably due to the external/manual procedure for pushes.
> 
> # Other issues
> 
> * It is difficult to follow the setup instructions.  For example,
>   <https://gnutoolchain-gerrit.osci.io/r/tools/hooks/commit-msg> for
>   the hook script is hard to find.  The documentation is outdated in
>   places.
> 
> * It does not make sense to provide a +2 and then point out typos and
>   request further changes.  The tool does not seem to prevent that.
> 
> * In the tool, there's no distinction between “this patch is
>   conceptually wrong” (i.e. no amount of polishing will overturn your
>   objection) and “this patch has bugs but I want to see it merged”.  I
>   think both are -2.  We haven't been very good at providing this kind
>   of feedback via email (it's difficult to turn down someone's
>   contribution), but Gerrit seems to encourage our bad practices in
>   this area.
> 
> * We would have to review many of the ACLs in the tool and somehow
>   model copyright assignment status.  For example, right now, I can't
>   add one of my patches to a topic after it has been pushed.
> 
> # Stuff I liked
> 
> * The indicator which patches have merge conflicts (but see above).
> 
> * There is a draft state for partially done reviews.
> 
> * The ability to cherry-pick individual patches with shell commands,
>   or checkout entire patch series is very nice.  It's how I submitted
>   my work to sourceware.org.  It easily beats what I can do with Gnus
>   today.
> 
> * Auto-closing pushed changes is really helpful, even with the manual
>   pushes we have today.
> 
>> Would it be wrong if we had two ways to submit patches?
> 
> Yes, see what happened with the manual update for strnlen etc.

I agree completely. The fact that it happened once is enough warning for me.

-- 
Cheers,
Carlos.