Re: Development practices / JIRA usage

[larry mccay]

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 
wrote:

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

Re: Development practices / JIRA usage

[Lars Francke]

Sorry, saw your new thread too late. Will reply there separately.

On Fri, Dec 11, 2015 at 3:29 PM, Lars Francke 
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]  >
>
>
>> On Fri, Dec 11, 2015 at 4:27 AM, Lars F

Re: Development practices / JIRA usage

[Lars Francke]

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] 


> On Fri, Dec 11, 2015 at 4:27 AM, Lars Francke 
> 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:
> >

Re: Development practices / JIRA usage

[larry mccay]

Thank you for your feedback, Lars.

Knox does follow a CTR policy for committers in Knox which is why you see
much of what you mention.
KNOX-640 was communicated on the dev@ list [1] to solicit feedback and
opinions.

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 will apply the patch, review the changes and provide comments back to the
contributor via JIRA comments.
Perhaps others have used review board for Knox.

Generally speaking, changes that are in line with current design, are bug
fixes, etc do not get reviewed.
Larger, architectural and security related changes should have patches
attached and get reviewed.


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.

I am aware of the Sentry thread and found it to be concerning and
unfortunate for that community, to be frank.
The incubator can certainly define what is required/desired for graduation
criteria though.

Our committer workflow process should probably be reworded to better
reflect the CTR policy of the Knox project.

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 will also look for well documented CTR policies within Apache and see how
well we align with those.

Again, thank you for your feedback. I hope that we can make you feel
comfortable enough to make more contributions to Knox. Please feel free to
express interest in any existing JIRAs, proposing features or whatever you
like.

[1]
http://mail-archives.apache.org/mod_mbox/knox-dev/201511.mbox/%3CCACRbFygWLh2xVXrEWAD5ae%2BdoLm7iKf3a3e8kOkWSnYKPMkj4g%40mail.gmail.com%3E

On Fri, Dec 11, 2015 at 4:27 AM, Lars Francke 
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] 
> [2] <
>
> http://thread.gmane.org/gmane.comp.apache.incubator.general/52126/focus=52351
> >
> [3]  >
> [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
> >
>