Re: development process

[Thomas Morley]

Am Mi., 5. Feb. 2020 um 00:12 Uhr schrieb David Kastrup <address@hidden>:
>
> Han-Wen Nienhuys <address@hidden> writes:

> >    The review process leaves changes in an unclear state. If Werner says
> >    LGTM, but then Dan and David have complaints, do I have to address the
> >    comments, or is change already OK to go in?

It's not a rare case, that someone gives a LGTM, overlooking serious weaknesses.
While someone else catches them.
Thus I think _all_ complaints should be addressed before pushing.

> The change is ok to go in when it has been approved for pushing.  I find
> the idea of ignoring instead of addressing complaints weird.

In general (without quoting other parts of previous mails) we should
decide: when is a patch mature for push?
(1) Currently a patch _without_ review-comments will end up in master
after the review-circle is finished.
This has the advantage that proposed code does not get lost.
Thus I like to keep it, although it comes with the risk that bugs are
introduced unnoticed.
As an example see:
buggy:
https://codereview.appspot.com/346030043/
fixed:
https://codereview.appspot.com/567060043

(2) As already said, if comments are present, all should be adressed.
Or a separate issue should be created. As an example:
- patch for new-feature xy is pushed
- documentation for it is delayed in issue <whatever>

>
> >    Rietveld and my local commits are not linked. If I change my commits, I
> >    update my commit message. I have to copy those changes out to Rietveld by
> >    hand, and it took me quite a while to find the right button (“edit 
> > issue”,
> >    somewhere in the corner).
>
> Then you have set it up incompletely or use it wrong.

David, I disagree. As far as I can tell git-cl does not update
commit-_messages_ on Rietveld.
Or I have an incomplete setup myself.

> >    The whole constellation of tools and processes (bug tracker for managing
> >    patch review, patch testing that seems to involve humans, Rietveld for
> >    patches, countdown, then push to staging) is odd and different from
> >    everything else. For contributing, you need to be very passionate about
> >    music typography, because otherwise, you won’t have the energy to invest 
> > in
> >    how the process works.

Yes, I took that route, fighting my my way through it.
Per aspera ad astra...

> >    The review tool supports painless reverts. When it is easy to fix up
> >    mistakes, contributors will feel less afraid to make changes.

Currently I can't imagine how this should work.

P.e. I once pushed a patch with a serious problem (not caught at review).
A  translator relied on it.
A fix for the initial problem and the translation-push resulted in
broken staging.

What would your suggestion to avoid such situations?

----

As already said frequently, our current method is suboptimal.
Afair, it was implemented because of the small amount of developers,
most of them with limited time.
As one example: the countdown-delay gave more of them the chance to
look at proposed patches...
Other problems: sourceforge is not the best as well, git-cl buggy etc

Han-Wen, I do understand that current setup slows you down and annoys
you and why.
Some other complaints you wrote could be summarized as not being
familar with those tools (e.g. Rietveld's edit).
I made similar expierences with GitHub, though:
I took me hours to figure out how to use oll-repo, it's all explained
in the wiki. I didn't find that wiki for a long time... grrrr
Or how to switch on GitHub between "unified diff" and "side-by-side
diff with in-line comments", took me felt ages ... grrr
To summarize: I'm really not familar with GitHub :)

Not being familar with a certain tool will vanish after some time ofcourse.
I still object to switch to GitHub, as said in Salzburg: GitHub is
nowadays owned by Microsoft.
But I would be open for other tools, if others agree (I can't comment
on GitLab or Gerrit, though, I simply have no experience with them).


My 2 cents,
  Harm