Larry, thanks a lot for the quick and detailed response.
Knox does follow a CTR policy for committers in Knox which is why you see > much of what you mention. > Okay, the Knox Wiki contradicts[1] what you're saying so that should probably be updated. There's been huge discussions about CTR vs RTC on the Incubator list as well. > KNOX-640 was communicated on the dev@ list [1] to solicit feedback and > opinions. > Sorry, yeah I just picked a random issue that happened to scroll by as an example here. > We watch the commits to the Apache git repository via the dev@ list and > easily review those changes post commit. > > I don't recall ever using review board for contributions in Knox. > I only mention it because someone from the Knox project requested it to be set up and it's used in other projects (e.g. Ambari) as well as reviews being mentioned in the wiki. Knox certainly has a number of external contributions from folks that > consist of bug fixes to larger features. > I think that it would be inaccurate to characterize such contributions as > blocked. However, we would love to encourage more. > It might have been better phrased on my end: Contributing new things is probably not blocked by any of this but getting feedback on changes that already happened certainly is. There is no good review process in the current Git setup (I mean a web interface with line by line annotations and multiple revisions etc.). I could provide feedback via Github but is that read? It's much easier to ask someone to create a Reviewboard review for something that I'd like to comment on before it's committed. Our committer workflow process should probably be reworded to better > reflect the CTR policy of the Knox project. > Ah, yes, agreed. Ignore my earlier comments on this then. > As for a 24 hr cool down period, I personally don't see the benefit to it > in a project that follows CTR. The patches are readily available via the > git notification and available for anyone to review. I do believe that as > the community gets larger that we will need to revisit the CTR/RTC policy > decision. > > All of that said, I have been consciously trying to communicate more via > dev@ and within the JIRAs - as you can see by [1] and KNOX-640 - which > reflect the content of that email thread. I think that we can discuss a > cool off period and the possible need for attaching patches for every > commit even though they are available through git. I'll start a separate > DISCUSS thread for those topics. I would urge you to provide your insights > - especially as to how the lack of either in a CTR process blocks external > contribution. > I totally understand that some/most of this is my personal preference or opinion and that others might disagree and that's fine. In my experience (mostly from Hive) things that have been committed don't get much attention even if someone raises concerns afterwards (especially if it's from a different company). This is especially true for minor things like code style violations or documentation. That is kinda understandable as most companies don't prioritise code cleanup/documentation etc. and there is no incentive to ever go back to modify these things. In my opinion it is much easier to fix issues before they have landed in the code base. I am a newcomer to Knox and I'm not going to lie: I probably won't contribute much if at all in the future just because of time constraints and usage of Knox at clients so I don't want to change policies that are working well for you. But I hope I have given you a perspective from an outsider. To me it was very weird digging into Knox issues which have nothing (sometimes not even a description) but a commit message. No +1, no patch etc. It's not how most projects work I work on (Hadoop, HBase, Hive, Spark, ...). I think a CTR model is fine in general but I think even a CTR model can include the _possibility_ of giving feedback before changes go in. And attaching a proposed patch and waiting for "a bit" is a huge part of it. I personally think that a cool off period is a good thing but I understand why you wouldn't want it. Attaching a patch though is incredibly helpful even long after a commit has gone in. Just assume that the Git url changes and all those links go stale or someone wants to run analysis which is way easier when dealing with standardised patch files than with a web interface. I frequently do the latter, download a patch and apply it to some local version to examine it. Again thanks a lot for your detailed and thoughtful response. Cheers, Lars [1] <https://cwiki.apache.org/confluence/display/KNOX/Contribution+Process> > On Fri, Dec 11, 2015 at 4:27 AM, Lars Francke <lars.fran...@gmail.com> > wrote: > > > Hi, > > > > I've been digging into Knox for the first time yesterday. I'm a > contributor > > to various other Apache projects and a Hive committer. > > > > I looked at a couple of JIRAs and then a few more random ones and I see a > > few concerning things: > > > > * Some/lots of tickets don't have the final patch attached and only point > > to a git or svn commit (KNOX-615, KNOX-601, KNOX-640) > > * I haven't seen a single review being done. Knox Reviewboard is > empty[1]. > > Reviews are hard to do when there's no patch to review anywhere :) > > * I found a couple of instances where patches were modified before > > committing without attaching the patch that's really been committed > > * I see issues being created, committed and closed within minutes ( > > https://issues.apache.org/jira/browse/KNOX-640) which does not really > > invite contributions and does not allow anyone to do any reviews or raise > > objections to issues. > > > > This last issue has recently been discussed at length in the Incubator > > mailing list in a thread called "Concerning Sentry"[2] and I urge you to > > read that. The suggestion in that thread was to wait at least 24h before > > committing a change and I think that's reasonable. > > > > Your contribution process[3] explicitly mentions that patches have to be > > attached to JIRA and that reviews are necessary. > > > > Just FYI I'm seeing the same practice in the Ambari project and raised > the > > topic there as well[4,5] > > > > I understand that you're doing this to make fast progress and you're not > > doing this to intentionally block external contributions but that's what > > the net effect may be. So I urge you to look at your development > practices > > and consider changing them. > > > > Thanks! > > > > Cheers, > > Lars > > > > [1] <https://reviews.apache.org/groups/Knox/> > > [2] < > > > > > http://thread.gmane.org/gmane.comp.apache.incubator.general/52126/focus=52351 > > > > > [3] < > https://cwiki.apache.org/confluence/display/KNOX/Contribution+Process > > > > > [4]< > > > > > http://mail-archives.apache.org/mod_mbox/ambari-dev/201512.mbox/%3CCAD-Ua_gt9TgKTxr12vrKO2kCEad3r97-GUwyB_LmUtFviFHt7A%40mail.gmail.com%3E > > > > > [5]< > > > > > http://mail-archives.apache.org/mod_mbox/ambari-dev/201512.mbox/%3CCAD-Ua_hpjfnYGF6HNuOQqTaUrqXqb0fc97BpqJ%3Dy%2BBw59csR4g%40mail.gmail.com%3E > > > > > >
