Am Mi., 5. Feb. 2020 um 00:12 Uhr schrieb David Kastrup <[email protected]>: > > Han-Wen Nienhuys <[email protected]> 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
