Terrific! I think that your points about the git web interface and stale links are good ones - please make sure that you add them to the DISCUSS thread.
On Fri, Dec 11, 2015 at 9:30 AM, Lars Francke <lars.fran...@gmail.com> wrote: > 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 > >> > > > >> > > >> > > > > >
