I see. The two are not the same, which is why I think they are possibly confusing. The “Committer Workflow using Git Branches” is about how to use remote git branches for feature development whereas “Committer Workflow” just uses a local git branch.
On 1/8/16, 10:53 AM, "larry mccay" <lmc...@apache.org> wrote: >Cool - thanks, Sumit. > >I am less concerned with it being confusing than with it being the same as >the previous section. > >On Fri, Jan 8, 2016 at 10:43 AM, Sumit Gupta <sumit.gu...@hortonworks.com> >wrote: > >> Hey Larry, thanks for the wiki update. It is clearly written and makes >> sense to me. >> >> The Committer Workflow with git Branches section is more of a capture of >> git commands than anything else and I can cleaned it up if it is >>confusing. >> >> Sumit. >> >> On 1/8/16, 9:35 AM, "larry mccay" <lmc...@apache.org> wrote: >> >> >All - >> > >> >Please review the contributing page updates that I just added. Not sure >> >how >> >to do drafts in the wiki... >> >https://cwiki.apache.org/confluence/display/KNOX/Contribution+Process >> > >> >Note the new section at the top called: Code Change Governing Policy >> > >> >This is the overall description or our policy, its definition, our >> >additional guidelines and our [REVIEW] notification mechanism. >> > >> >I also made minor language problems to align with this policy >>throughout >> >the document. >> > >> >I notice that the Committer Workflow and Committer Workflow with git >> >Branches seem redundant. Is there any meaningful difference between the >> >two? >> >Given that most of our commits do not use feature branches, perhaps we >> >should change the first one to be patch based instead? >> > >> >thanks, >> > >> >--larry >> > >> >On Fri, Jan 8, 2016 at 8:43 AM, larry mccay <lmc...@apache.org> wrote: >> > >> >> Great - I will spend some time today putting something together that >>we >> >> can iterate over. >> >> >> >> On Thu, Jan 7, 2016 at 2:29 PM, Sumit Gupta >> >><sumit.gu...@hortonworks.com> >> >> wrote: >> >> >> >>> +1. I’m good with this. >> >>> >> >>> >> >>> On 1/7/16, 12:42 PM, "larry mccay" <larry.mc...@gmail.com> wrote: >> >>> >> >>> >Okay - let's bring this to a close - with a slight clarification on >> >>>the >> >>> >[REVIEW] emails... >> >>> > >> >>> >As long as we are all in agreement on the following, I will >>attempt to >> >>> >document it as our contribution policy (with a follow up email for >>the >> >>> >draft): >> >>> > >> >>> >* Knox has a CTR policy for committers >> >>> >* For any patches that make fundamental architectural or security >> >>>related >> >>> >changes - committers should solicit feedback on the design >> >>> >* For any patches that are extensions to existing patterns for >> >>>features - >> >>> >such as adding new service API support, the committer may commit >> >>>freely - >> >>> >given sufficient tests and documentation (to the committer's >> >>>discretion) >> >>> >* For any patches that are simple bug fixes, the committer may >>commit >> >>> >freely >> >>> > >> >>> >* In order to facilitate reviews from other community members post >> >>> commit, >> >>> >we will use an email with a [REVIEW] tag to indicate that comments >>are >> >>> >being provided for a merged change and that it needs attention. >> >>> > >> >>> >There is nothing saying that we can't revisit the CTR policy in the >> >>> future >> >>> >and we will continue to leverage the contributions of the >>community at >> >>> >large. If at any point, any of our development process seems to be >> >>> barrier >> >>> >for contributors we will need to be open to change. >> >>> > >> >>> >@Sumit - note that the above does not describe a [REVIEW] email for >> >>>every >> >>> >feature. >> >>> > >> >>> >Features continue to be described in JIRA and designs in attached >> >>> >documents >> >>> >or wiki. >> >>> >Reviews should also be conducted in JIRA. >> >>> >Reviews for something already committed and the JIRA is closed can >> >>>have a >> >>> >[REVIEW] email as a flag to the committers to give attention to the >> >>> >feedback. >> >>> > >> >>> >If everyone is good with the above, I will draw up a draft. >> >>> > >> >>> >On Thu, Jan 7, 2016 at 11:42 AM, Sumit Gupta < >> >>> sumit.gu...@hortonworks.com> >> >>> >wrote: >> >>> > >> >>> >> Sorry for coming late to the discussion. Like Kevin, I seem to be >> >>> >> constantly missing email from the dev list (the email server >>doesn¹t >> >>> >>like >> >>> >> me). >> >>> >> >> >>> >> I¹m not sure how having a CTR policy for most things and then a >> >>> >>selective >> >>> >> RTC policy helps drive community involvement. It seems hard to >> >>> >>implement. >> >>> >> If anything, I can see more [DISCUSS] threads and like Larry >> >>>suggested >> >>> a >> >>> >> [REVIEW] email for all features. This would help awareness as >>well >> >>>as >> >>> be >> >>> >> useful in obtaining critical feedback. I would have preferred a >>JIRA >> >>> >> mechanism for asking for a review since we get JIRA emails on the >> >>>dev >> >>> >>list >> >>> >> anyway, but I can see a review tag or comment getting lost in >>swarm >> >>>of >> >>> >> JIRA email. >> >>> >> >> >>> >> In short, I would prefer a single straightforward policy of CTR >>and >> >>> >>other >> >>> >> means of raising awareness and soliciting input. >> >>> >> >> >>> >> >> >>> >> On 1/7/16, 8:40 AM, "larry mccay" <lmc...@apache.org> wrote: >> >>> >> >> >>> >> >I am not looking to change our policy at this point but instead >>to >> >>> >>define >> >>> >> >the expectations of CTR in the Knox community - largely based on >> >>>what >> >>> >>has >> >>> >> >been done all along unofficially. The [REVIEW] email to dev@ >>seems >> >>>a >> >>> >> >decent >> >>> >> >way to raise attention to a review for committed code. >> >>> >> > >> >>> >> >On Thu, Jan 7, 2016 at 12:15 AM, Kevin Minder >> >>> >> ><kevin.min...@hortonworks.com> >> >>> >> >wrote: >> >>> >> > >> >>> >> >> So in short: Bugs==CTR, Features==RTC? >> >>> >> >> >> >>> >> > >> >>> >> >I don't know that Features==RTC is required for extensions of >> >>>existing >> >>> >> >patterns. >> >>> >> >For instance, adding support for a new service shouldn't require >> >>>RTC. >> >>> >> > >> >>> >> >This does bring something to mind though. If there is >>complicated >> >>> >>custom >> >>> >> >HA >> >>> >> >related dispatch code then that should probably be documented or >> >>> >> >communicated to the dev@ and it would be up to the committer to >> >>> >>determine >> >>> >> >whether they would like to ask for a review. >> >>> >> > >> >>> >> >Understanding the HA dispatch algorithms for services by the dev >> >>> >>community >> >>> >> >or at least the ability to get to an understanding through >>readily >> >>> >> >available documentation. >> >>> >> > >> >>> >> > >> >>> >> >> >> >>> >> >> From: larry mccay >><lmc...@apache.org<mailto:lmc...@apache.org>> >> >>> >> >> Date: Wednesday, January 6, 2016 at 11:06 PM >> >>> >> >> To: Kevin Minder <kevin.min...@hortonworks.com<mailto: >> >>> >> >> kevin.min...@hortonworks.com>> >> >>> >> >> Cc: "dev@knox.apache.org<mailto:dev@knox.apache.org>" >> >>> >> >><dev@knox.apache.org >> >>> >> >> <mailto:dev@knox.apache.org>> >> >>> >> >> Subject: Re: [DISCUSS] CTR, Requiring Patch Attachments and >>Cool >> >>>Off >> >>> >> >>Period >> >>> >> >> >> >>> >> >> Hi Lars - >> >>> >> >> >> >>> >> >> Thanks for bumping this thread again - we do need to bring it >>to >> >>>a >> >>> >> >>close. >> >>> >> >> >> >>> >> >> I certainly agree with Kevin on CTR as the current and well >> >>>working >> >>> >> >>policy >> >>> >> >> for this project. >> >>> >> >> >> >>> >> >> @Kevin - I believe that we need to also consider the cool off >> >>>period >> >>> >> >>yet. >> >>> >> >> I do not believe that external contributions are being >>blocked by >> >>> the >> >>> >> >>lack >> >>> >> >> of a cool off period. >> >>> >> >> >> >>> >> >> We have numerous externally contributed features, >>enhancements to >> >>> >> >>existing >> >>> >> >> features and bug fixes. >> >>> >> >> >> >>> >> >> Sorry to hear that you have had bad experiences with other >> >>>projects >> >>> >> >> ignoring reviews. >> >>> >> >> Let me make sure that I understand what you describe... >> >>> >> >> >> >>> >> >> I think that you are saying that you provided review comments >>and >> >>> >> >> suggestions to someone's patch for some apache project and >>that >> >>>the >> >>> >> >>review >> >>> >> >> comments were never responded to and the patch was committed >> >>>without >> >>> >> >> acknowledgement. Maybe you provided a review for code that was >> >>> >>already >> >>> >> >> committed and it was ignored. >> >>> >> >> >> >>> >> >> I can see the review being easier to miss/ignore when the code >> >>>and >> >>> >>JIRA >> >>> >> >>is >> >>> >> >> already committed and closed. >> >>> >> >> >> >>> >> >> My proposal is that we document the following (most of which >>is >> >>>from >> >>> >>my >> >>> >> >> original email): >> >>> >> >> >> >>> >> >> * Changes, that are aligned with existing design patterns and >> >>>Knox >> >>> >> >> architecture, that are either incremental >>enhancements/features >> >>>or >> >>> >>bug >> >>> >> >> fixes can be purely CTR >> >>> >> >> * Changes that necessitate architectural changes, have >> >>>significant >> >>> >> >> security implications, or are generally large should be >>reviewed. >> >>> >>This >> >>> >> >>is >> >>> >> >> to facilitate communication of such changes as well as to have >> >>> >>another >> >>> >> >>set >> >>> >> >> of eyes for the code. >> >>> >> >> * Architectural changes and completely new features should be >> >>> >> >>communicated >> >>> >> >> via the dev@ list and possibly within a wiki page for design >> >>> >> >> discussion/communication. >> >>> >> >> * Review feedback for patches that have already been committed >> >>>and >> >>> >>JIRAs >> >>> >> >> closed/resolved should also be sent to the dev@ list with a >> >>> [REVIEW] >> >>> >> tag >> >>> >> >> in the subject. Committers will need to assess the points >>raised >> >>>and >> >>> >> >> respond. Optionally, based on discussion the JIRA can be >> >>>reopened, >> >>> >> >>possibly >> >>> >> >> commit reverted or new JIRAs filed to address the reviewer's >> >>> >>feedback. >> >>> >> >> >> >>> >> >> I think that this will fully communicate our development >>process >> >>>and >> >>> >> >> provide rules of engagement for post commit reviews. >> >>> >> >> >> >>> >> >> Let me know your thoughts on this plan. >> >>> >> >> >> >>> >> >> thanks, >> >>> >> >> >> >>> >> >> --larry >> >>> >> >> >> >>> >> >> On Wed, Jan 6, 2016 at 10:31 PM, Kevin Minder < >> >>> >> >> >> >>>kevin.min...@hortonworks.com<mailto:kevin.min...@hortonworks.com>> >> >>> >> >>wrote: >> >>> >> >> First apologies. Seems my mail server has been randomly >>eating >> >>> >>Apache >> >>> >> >> mail. I didn¹t see the original mail here. >> >>> >> >> >> >>> >> >> First point, attached patches. I used to attach patches to >>all >> >>>my >> >>> >>jira. >> >>> >> >> Then I realized that a) the jira had links with diffs for the >> >>>lazy >> >>> >>and >> >>> >> >>³git >> >>> >> >> format-patch <commit-sha>² will generate the patch for those >>that >> >>> >>want >> >>> >> >>to >> >>> >> >> use their favorite tool on a local patch file. At that point >> >>> >>requiring >> >>> >> >> patch generation from committers for a CTR project just seemed >> >>>like >> >>> >> >>process >> >>> >> >> for the sake of process. This being said we could do a better >> >>>job >> >>> of >> >>> >> >> documenting this for those unfamiliar with git if Larry hasn¹t >> >>> >>already >> >>> >> >> taken care of that. >> >>> >> >> >> >>> >> >> Second point, CTR. We have been CRT from the beginning and >>this >> >>> >>model >> >>> >> >> certainly made sense given the rapid pace of development. >>Things >> >>> >>have >> >>> >> >> certainly slowed down recently and we can continue the >> >>>discussion. >> >>> >> >> However, the core of the question you are really asking is >>would >> >>> more >> >>> >> >> people contribute if we were RTC. I don¹t think so. We >>fairly >> >>>bend >> >>> >>of >> >>> >> >> backwards to embrace traffic on user@knox and dev@knox. I >>think >> >>> that >> >>> >> is >> >>> >> >> far more significant than a CTR vs RTC policy. >> >>> >> >> >> >>> >> >> >> >>> >> >> >> >>> >> >> On 1/6/16, 7:49 PM, "Lars Francke" >> >>><lars.fran...@gmail.com<mailto: >> >>> >> >> lars.fran...@gmail.com>> wrote: >> >>> >> >> >> >>> >> >> >Does anyone have anything further to add here? Looking >>forward >> >>>to a >> >>> >>few >> >>> >> >> >more opinions. >> >>> >> >> > >> >>> >> >> >On Fri, Dec 11, 2015 at 3:52 PM, Lars Francke >> >>> >><lars.fran...@gmail.com >> >>> >> >> <mailto:lars.fran...@gmail.com>> >> >>> >> >> >wrote: >> >>> >> >> > >> >>> >> >> >> Hi, >> >>> >> >> >> >> >>> >> >> >> thanks Larry. >> >>> >> >> >> >> >>> >> >> >> Lars provided the Knox community with some feedback into >>our >> >>> >> >>development >> >>> >> >> >>> practices and JIRA usage [1]. >> >>> >> >> >>> >> >>> >> >> >>> I wanted to bring up a DISCUSS thread on how our CTR >>policy >> >>>may >> >>> >>or >> >>> >> >>may >> >>> >> >> >>> not relate to a couple points made in his feedback. In >> >>> >>particular: >> >>> >> >> >>> >> >>> >> >> >>> 1. Whether a CTR based policy should require actual >>patches >> >>> >> >>attached to >> >>> >> >> >>> every JIRA or does the git link to a commit provide the >>same >> >>> >> >>ability to >> >>> >> >> >>> review post commit >> >>> >> >> >>> >> >>> >> >> >> >> >>> >> >> >> I think having a patch attached before commit is a good >>thing >> >>> >> >>because: >> >>> >> >> >> * It allows feedback before a change goes in. In my >>experience >> >>> >>that >> >>> >> >>is >> >>> >> >> >> easier to change than committed code >> >>> >> >> >> * It allows looking at the exact change set in a >>standardised >> >>> form >> >>> >> >>(diff >> >>> >> >> >> format) without having to use whatever web frontend (pure >>Git, >> >>> >> >>Github, >> >>> >> >> ...) >> >>> >> >> >> is currently being used (so a patch file is useful even >>when >> >>>only >> >>> >> >> attached >> >>> >> >> >> after committing)[1] >> >>> >> >> >> * Should the Git web interface change (or be down) at some >> >>>point >> >>> >>in >> >>> >> >>the >> >>> >> >> >> future all those links might go stale (say Apache switches >>to >> >>> >>Github >> >>> >> >>or >> >>> >> >> >> Gitlab or whatever) >> >>> >> >> >> >> >>> >> >> >> The downsides I can come up with is the extra work >>required to >> >>> >>attach >> >>> >> >> the >> >>> >> >> >> patch (before or after commit). >> >>> >> >> >> >> >>> >> >> >> >> >>> >> >> >>> 2. Whether a cool off period of 24 hrs would encourage >>more >> >>> >>external >> >>> >> >> >>> contributions and if so, how >> >>> >> >> >>> >> >>> >> >> >> >> >>> >> >> >> This one is probably a matter of weighing off between fast >> >>> >>iteration >> >>> >> >>and >> >>> >> >> >> possible feedback on patches. I assume with a project the >> >>>size of >> >>> >> >>Knox >> >>> >> >> >> there is not much feedback coming in for each patch so the >> >>> >>benefits >> >>> >> >>of >> >>> >> >> >> immediate commits probably outweigh the (assumed) benefits >>of >> >>> >> >>waiting. >> >>> >> >> >> >> >>> >> >> >> My personal opinion though is that a wait period is a good >> >>>thing. >> >>> >> >>I've >> >>> >> >> >> been bitten (and frustrated) in the past by reviews that >>have >> >>> been >> >>> >> >> ignored >> >>> >> >> >> by certain communities/members/companies where it was clear >> >>>that >> >>> a >> >>> >> >> release >> >>> >> >> >> schedule had to be met. Ignoring reviews is easier with >>code >> >>> >>that's >> >>> >> >> already >> >>> >> >> >> been committed. Again this is my personal (bad) experience >> >>>and I >> >>> >> >>have no >> >>> >> >> >> reason to believe that the Knox community behaves the >>same. A >> >>> cool >> >>> >> >>off >> >>> >> >> >> period doesn't mean that reviews are mandatory, it just >> >>> >> >>invites/allows >> >>> >> >> >> feedback in my opinion. >> >>> >> >> >> >> >>> >> >> >> One "real" benefit for pre-commit reviews is that there's >> >>>tools >> >>> >> >> available >> >>> >> >> >> and in use at Apache for that (Reviewboard and JIRA to a >> >>>degree) >> >>> >>but >> >>> >> >> >> there's currently no tooling support for post-commit >>reviews. >> >>> >> >> >> >> >>> >> >> >> Caveat to all of this: I'm only here for a few days a year >> >>> >>probably >> >>> >> >>and >> >>> >> >> >> won't contribute much if at all so you should decide >>carefully >> >>> >> >>whether >> >>> >> >> you >> >>> >> >> >> want to change working practices for an unknown benefit. >> >>> >> >> >> >> >>> >> >> >> Cheers, >> >>> >> >> >> Lars >> >>> >> >> >> >> >>> >> >> >> [1] I know that the current web interface has an option to >> >>> >>download >> >>> >> >>the >> >>> >> >> >> patch but it's a different process than most other "Hadoop >> >>> >>related" >> >>> >> >> >> projects. >> >>> >> >> >> >> >>> >> >> >> >>> >> >> >> >>> >> >> >>> >> >> >>> >> >>> >> >> >> >>
