Sorry, saw your new thread too late. Will reply there separately. On Fri, Dec 11, 2015 at 3:29 PM, Lars Francke <lars.fran...@gmail.com> wrote:
> 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 >> > > >> > >> > >
