This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: FYI: Status of gdb usage of gerrit


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

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]