On the need to move to a merge request workflow

[Daniel P. Berrangé]

On Fri, Mar 27, 2020 at 02:15:01PM +0100, Peter Krempa wrote:
> On Thu, Mar 26, 2020 at 15:50:20 +0000, Daniel Berrange wrote:
> > On Thu, Mar 26, 2020 at 02:10:59PM +0100, Peter Krempa wrote:
> > > On Fri, Mar 06, 2020 at 11:44:07 +0000, Daniel Berrange wrote:
> > > > We've discussed the idea of replacing our mailing list review workflow with
> > > > a merge request workflow in various places, over the last 6 months or so,
> > > 
> > > One thing I feel the need to voice until this is taken in place is a
> > > matter of personal preference:
> > > 
> > > I severely dislike the merge request workflow and I'll be severely
> > > disappointed once we switch over to it.
> > 
> > Can you elaborate on specific things you don't like, so we can see if
> > there are any options to mitigate them ?
> 
> - it's a web page
>     - it's very slow on big projects and slow in general
>     - it's ugly, "theme" support is a joke, white background burns my
>       eyes
>     - it works badly on a portrait display
>     - mangles commit messages in an attempt to render them (link and
>     signed-off line concatenated:
>     https://gitlab.com/libvirt/libvirt/-/commit/e05dd1abdc3b3eeac6e12ab105e56138d783af2a

This looks like a bug in their code which renders the messages. It is
failing to preserve a newline character when it follows a hyperlink.
I'll see about reporting this as an issue.

> 
> 
> - usability
>     - default view after opening a branch is "Files" not "commits"
>     - web interface doesn't really carry information about which merge
>       requests are new to me
> 
> - review is terrible
>     - threads are only single level
>     - response can't be quoted (okay, they can but you have to copy and
>       pase what you want to quote and then select that it's a quote)
>     - you need to click to see the code
>     - comments to code are in proportional font
>     - a lot of useless clutter in the UI
>     - need to click open comments in code
>     - extra steps necessary to apply code locally (granted you get a
>       branch by default, but fetching it is not as easy as I have with
>       mail workflow)

Overall the issues are primarily focused on the web UI, rather than
the conceptual idea of the merge request workflow.  I share the same
view of many of these issues, but I do think we can mitigate most of
it with a terminal based tool as an option for those who don't like
the web UI.


>     - review process favours review without actually fetching the code
>       locally (if you can click a button, who will actually test that
>       the code works?)

If anything I feel the current email review makes me even less likely
to actually apply & test the code locally, as it requires several tedious
error prone steps, compared to being able to direct "git fetch" a ref
for the merge request.

The more automated testing we do the better, but either way we rely on
reviewers to be diligent enough to decide when something needs some
more testing. 

> 
> 
> Now I know you will suggest I try your 'bichon' tool for reviews as it
> fixes a handful of the problems outlined above, but unfortunately that
> comes with it's own set of problems since it's in infancy stage.

Yes, it certainly isn't developed to where it needs to be yet, but I have
started using it for real code review on libosinfo repos and found it is
useful already.

> In addition with current upstream bug tracker and discussions at least
> being delivered using email I have notifications about new stuff even if
> I have to interact with a web page. Bichon doesn't seem to want to deal
> with 'issues' section at all.

GitLab issues can be delivered via email just like Bugzilla, though IIRC
you'll need to opt-in for any repositories you care about (if you're not
the repo owner). In the main page for a repo

  https://gitlab.com/libvirt/libvirt/

Next to the repo title there is a bell icon where you can control
what level of email notifications you get.

Your profile page:

   https://gitlab.com/profile/notifications

will show what repos you're getting notifications on currently

Fabiano did file an RFE suggesting that we should support the issue
tracker in Bichon too.  I think that is a sensible idea, but personally
I'm going to focus on merge requests right now so that's where the
biggest wins will be.  In terms of Web UI for issues, it is already
better than Bugzilla as it is way simpler without all the RHEL tracking
junk.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|