So in short: Bugs==CTR, Features==RTC? 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. >>
