+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. >> >> >> >> >> >> >> >> >>
