I'm straight down the middle on this one. I don't object to either. I don't prefer either of them. I've seen both contribute to quagmire and hurt feelings, and both work just perfectly well.
On Wed, Dec 10, 2014 at 9:34 PM, Mark Payne <[email protected]> wrote: > I think the big question, which you brought up, is "what constitutes a > review?" If another committer looking over the code and signing off is > sufficient then I'm not horribly opposed, though I think it will VERY MUCH > impede the speed at which any new features / bug fixes can be accomplished. > > If we are talking about waiting for several people to vote, then I think > it's an absolute non-starter and destroys any hope of moving even at a > fraction of the rate we are used to. This is especially concerning if we > are waiting for a consensus for something like a trivial bug fix like an > NPE. > > Thanks > -Mark > > > On Dec 10, 2014, at 9:20 PM, Joe Witt <[email protected]> wrote: > > > > NiFi grew up in a CT[R] environment where the review was very rare and so > > really we ended up with CTDTFSOR - Commit then deploy then feel sense of > > regret. > > > > So the bad side: We ended up with a lot of avoidable defects where by > > avoidable I mean they could have likely been detected in a decent review > > effort. > > > > The good side: We were fast. Very fast. New features got out quick and > if > > something was funky we fixed it very fast. For the vast majority of > > mistakes the problem was highly isolated (thanks to our base design > > construct of FBP) and rarely caused extremely stressful days. > > > > On balance though and given the benefits it has to community growth I > > personally am very supportive of us adopting RTC immediately provided we > > can sort out some key questions: > > > > - What constitutes a valid review? > > This page > http://www.apache.org/foundation/glossary.html#ReviewThenCommit > > suggests that a review is a consensus approval. I am very concerned by > > that definition if we're talking every commit means we need a vote. For > > the vast majority of commits this just seems to onerous and too time > > consuming and frankly to me takes some of the fun out of developing. If > > for us a review is simply that a 'committer' has reviewed the code then I > > am perfectly happy with this and I am thinking this is in-line with the > > model Benson described for his dayjob "'make branch, submit pr, get > review, > > merge'" > > > > - What is the best tooling/process around this to actually > conduct/document > > the review? > > I think > > > https://cwiki.apache.org/confluence/display/KAFKA/Patch+submission+and+review#Patchsubmissionandreview-Simplecontributorworkflow > > does a good job of describing the mechanics of a review/submission > > process. And I'd just assume a patch is as good as a pull request. I > > think even as a good starting point having the JIRA ticket commented on > by > > the reviewer would be good. > > > > - What are some key things a reviewer should be checking for and helping > to > > enforce? What are some out of bounds things? > > > > > > > > This seems like a really important discussion and something that we > should > > center on sooner than later. There are ways to nuance this such as RTC > for > > the core and CTR for extensions and so on but I think that just leads to > > more confusion. We should likely pick our poison. I see strong benefits > in > > both directions and so I can deal either way personally. I see a couple > > strong arguments here in this thread already for RTC . > > > > Anyone willing to argue for CTR? > > > > Thanks > > Joe > > > > > > On Wed, Dec 10, 2014 at 8:21 PM, Benson Margulies <[email protected] > > > > wrote: > > > >> It seems to me, and this is purely opinion, that RTC has a lot to be > said > >> for a new project, in terms of getting more people involved, more > eyeballs, > >> and more of a community feeling. My dayjob workflow is 'make branch, > submit > >> pr, get review, merge' and that seems pretty applicable. However, I am > not > >> wearing any magic mentor hat-o-authority. > >> > >>> On Wed, Dec 10, 2014 at 2:23 PM, Billie Rinaldi <[email protected]> > wrote: > >>> > >>>> On Tue, Dec 9, 2014 at 8:07 AM, Joe Witt <[email protected]> wrote: > >>>> > >>>> Benson - thanks for the headsup on the maven plugin. Seems like using > >>> them > >>>> to their fullest capability and then manually merging to master is > >>>> perfectly fine to me. > >>>> > >>>> Billie > >>>> As for CTR i don't think i have a good enough appreciation for the > >>>> process/value proposition here. Curious of other folks views. > >>> > >>> Both CTR and RTC have their own advantages, so either would be fine ( > >>> http://www.apache.org/foundation/glossary.html#CommitThenReview). In > >>> retrospect I see my question could be viewed as leaning towards CTR, > but > >> it > >>> was just based on an observation that people already seemed to be > making > >>> commits without review. > >>> > >>> > >>>> > >>>> It seems reasonable to keep the feature branch relaxed and in that > >> sense > >>>> what Gilman did today seems fine. We can get bettter at those > >> judgement > >>>> calls and documenting the criteria as we go along. > >>>> > >>>> Thanks > >>>> Joe > >>>> > >>>> > >>>> > >>>> On Tue, Dec 9, 2014 at 11:00 AM, Benson Margulies < > >> [email protected] > >>>> > >>>> wrote: > >>>> > >>>>> PR's are certainly convenient. There's no much difference, for a > >>>> committer, > >>>>> between pushing a branch to the official repo and pushing a branch to > >>>> some > >>>>> personal repo on github. The same integration workflow can be used > >>> either > >>>>> way to close out the PR upon merge. > >>>>> > >>>>> However, in a CTR project, it seems perhaps excessive to _require_ > >>>> feature > >>>>> branches for small fixes as opposed to just committing them directly > >> to > >>>>> develop. At day job we do mostly do branch-per-jira, but some people > >>>> might > >>>>> find that onerous. > >>>>> > >>>>> Pushing to the official repo leaves more history that someone might > >>> find > >>>>> interesting some day. Also more clutter; some people are very > >> concerned > >>>>> about repacking before merging to develop. > >>>>> > >>>>> Another issue with gitflow is the master branch. The master branch is > >>>>> supposed to get merged to for releases. The maven-release-plugin > >> won't > >>> do > >>>>> that, and the jgitflow plugin is unsafe. So one option is to 'use > >>>> gitflow' > >>>>> but not bother with the master versus develop distinction, the other > >> is > >>>> to > >>>>> do manual merges to master at release points. > >>>>> > >>>>> > >>>>> On Tue, Dec 9, 2014 at 10:22 AM, Joe Witt <[email protected]> > >> wrote: > >>>>> > >>>>>> Hello > >>>>>> > >>>>>> So a question on gitflow given that commits are now underway. When > >>>>> working > >>>>>> on a feature in a local repo which is a branch off the develop > >>>>> branch...do > >>>>>> you push the feature branch to the central repo? This then can be > >>>> merged > >>>>>> by someone else as a sort of code review/pull process? > >>>>>> > >>>>>> Thanks > >>>>>> Joe > >>>>>> > >>>>>> On Mon, Dec 8, 2014 at 11:40 PM, Joe Witt <[email protected]> > >>> wrote: > >>>>>> > >>>>>>> All, > >>>>>>> > >>>>>>> Now that we have our code up it is important to establish a > >> process > >>>>>> around > >>>>>>> git in particular. A general consensus in the thread appears to > >> be > >>>>> that > >>>>>>> gitflow workflow is a reasonable option. > >> > https://www.atlassian.com/git/tutorials/comparing-workflows/gitflow-workflow > >>>>>>> > >>>>>>> To that end I've added a develop branch off of master from which > >>>>> features > >>>>>>> can be built. As we converge toward a release then we'll > >>>>>> address/introduce > >>>>>>> some of the other aspects of gitflow. > >>>>>>> > >>>>>>> Please discuss/comment if there are views that we should be > >> taking > >>>>>> another > >>>>>>> path. > >>>>>>> > >>>>>>> Thanks > >>>>>>> Joe > >>>>>>> > >>>>>>> On Sat, Nov 29, 2014 at 8:16 AM, Benson Margulies < > >>>>> [email protected] > >>>>>>> > >>>>>>> wrote: > >>>>>>> > >>>>>>>> On Sat, Nov 29, 2014 at 3:45 AM, Sergio Fernández < > >>>> [email protected]> > >>>>>>>> wrote: > >>>>>>>>> Hi Adam, > >>>>>>>>> > >>>>>>>>> one remarks about this: > >>>>>>>>> > >>>>>>>>>> On 28/11/14 18:07, Adam Taft wrote: > >>>>>>>>>> > >>>>>>>>>> Knowing how we work today, if it were me, I would suggest > >> using > >>>> the > >>>>>>>> above > >>>>>>>>>> workflow combined with the "forking workflow" to guard access > >>> to > >>>>> the > >>>>>>>>>> production release (master) branches. A very small subset of > >>> the > >>>>>>>>>> incubator's commiters should have the ability to merge the > >>>>> "develop" > >>>>>>>>>> branch > >>>>>>>>>> down to a master "release" branch. > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> Suck workflow is not in place in ASF. On the one hand, the > >>> current > >>>>> git > >>>>>>>>> infrastructure does not provide such branches' management, > >> like > >>>>>>>>> bitbucket/stash do for instance. On the other hand, and more > >>>>>> important, > >>>>>>>> a > >>>>>>>>> project is not hierarchical organization, but a a meritocratic > >>>> one. > >>>>>>>>> > >>>>>>>>> I recommend you this blog post in case you want to read a bit > >>>> more: > >> http://communityovercode.com/2012/05/meritocracy-and-hierarchy/ > >>>>>>>>> > >>>>>>>>> If someone has permissions to do (i.e., he is a committer), he > >>> can > >>>>> do > >>>>>>>> it, > >>>>>>>>> simple The tool provide you instruments to revert those > >> changes > >>> in > >>>>>> case > >>>>>>>> on > >>>>>>>>> involuntary errors. > >>>>>>>>> > >>>>>>>>>> It would be ideal to have someone who > >>>>>>>>>> is NOT performing the majority of changes on the "develop" > >>> branch > >>>>>> take > >>>>>>>>>> this > >>>>>>>>>> role to coordinate releases, ensure minimal coding standards, > >>> run > >>>>>>>> through > >>>>>>>>>> unit and integration tests, before signing off on the release > >>> and > >>>>>>>> issuing > >>>>>>>>>> the release artifacts. > >>>>>>>> > >>>>>>>> You seem to be imagining an individual with a job which is > >> shared > >>> in > >>>>>>>> by the community. In healthy communities, a release happens when > >>>>>>>> there's a consensus to have a release. There is no person who > >>>> 'ensures > >>>>>>>> minimal coding standards', that's everyone watching commits. > >>> There's > >>>>>>>> no one 'running unit and integration tests' because (a) every > >>>>>>>> committer does this before every commit, (b) Jenkins does it, > >> (c) > >>>> the > >>>>>>>> release process does it. (d) there's no signing off on a > >> release. > >>>> The > >>>>>>>> RM puts it up for a vote, and PMC members vote. > >>>>>>>> > >>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> Release comes after that. The release manager is responsible > >> on > >>>>>>>> creating a > >>>>>>>>> proper release, which must include a code release and should > >>>> include > >>>>>>>>> binaries too. Each artifact release must be signed. > >> Demonstrate > >>>> your > >>>>>>>> ability > >>>>>>>>> as a project to produce releases is one of the goals of the > >>>>>> incubation. > >>>>>>>> But > >>>>>>>>> we are not yet there, step by step. > >>>>>>>>> > >>>>>>>>> Hope that helps. > >>>>>>>>> > >>>>>>>>> Cheers, > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> -- > >>>>>>>>> Sergio Fernández > >>>>>>>>> Partner Technology Manager > >>>>>>>>> Redlink GmbH > >>>>>>>>> m: +43 660 2747 925 > >>>>>>>>> e: [email protected] > >>>>>>>>> w: http://redlink.co > >> >
