On Thu, Apr 3, 2014 at 10:02 PM, Sravya Tirukkovalur
<[email protected]> wrote:
> David,
>
> Thanks for the feedback!
>
> Points raised and comments below:
>
> 1. License headers:
> Looks like the apache-rat plugin is part of verify phase:
> https://github.com/apache/incubator-sentry/blob/master/pom.xml#L436
> So it would not be active and hence does not exclude necessary files if you
> do $mvn apache-rat:check directly. Although mvn verify, does the rat check
> successfully and passes.


Sorta - first you don't have excludeSubProjects set to false, which
means it's not checking all of the sub projects. Secondly the excludes
are pretty broad; which isn't an inherent problem, but expect someone
on general@ to ask why there are so many files without headers. rat is
merely a tool, doesn't obviate the need to establish provenance or
comply with the policy on having license declared in files.

>
> 2. Name of archive:
> Should be fixed.
>

Easy fix too - just rename the file and the hash and sigs, things
still work out.


> 3. Compile error:
> Looks like this is due to a maven bug, that it is trying to resolve test
> scoped dependencies in compile time where it doesnt build test artifacts:
> http://stackoverflow.com/questions/4786881/why-is-test-jar-dependency-required-for-mvn-compile
> We may add this to our README.
>

It is indeed - and should go away once things artifacts published to
the maven repo; but is annoying in the meantime.


> 4. Test error:
> Karthik, you are right that wget is needed, we should probably add this to
> README?
>

It is - and my fault for not looking at the actual error. After the
build error in compile I stopped investigating and just copy/pasted.


> 5. Tag:
> If I understand it right, we should have created a tag release-1.3.0
> instead of release-1.3.0-rc1?

That's what I'd have done.
Also the other point is that you need to communicate the hash because
the tags are not immutable.

>
> Thanks!
>
> On Thu, Apr 3, 2014 at 5:25 PM, karthik ramachandran <
> [email protected]> wrote:
>
>> David,
>>
>> With regard to the errors you're encountering I saw the same thing the
>> first time I tried to build.
>> Turned out that I didn't have wget installed.
>>
>> Since I was building on OS X, at the time, I simply did brew install wget.
>>  Not sure about other platforms.
>>
>> Thanks - for the rest of the feedback.
>>
>>
>> On Thu, Apr 3, 2014 at 5:15 PM, David Nalley <[email protected]> wrote:
>>
>> > Karthik:
>> >
>> > Please don't forget to include a hash for the tag - tags aren't
>> > immutable in git. As a matter of fact, in the future, I'd propose that
>> > you actually make the tag release-1.3.0 and delete the tag if it
>> > doesn't pass.
>> >
>> > Sigs are fine, hashes are fine.
>> > License, Notice, and Disclaimer look fine.
>> >
>> > There are a number of files that don't have license headers. I also
>> > suspect that many didn't originate with Sentry (Velocity templates are
>> > a number of them) These seem to have been added since 1.2.0 went out.
>> > Has anyone run RAT against this? But we need to have an explanation
>> > for this at a minimum, so we can document provenance of the files.
>> >
>> > Name of the archive is problematic[1]. The name must be
>> > $project-$version-incubating.tar.gz though
>> > apache-$project-$version-incubating.tar.gz is even better. Ironically
>> > it expands into the proper naming convention.
>> >
>> > Being the nascent user of Sentry that I am I proceeded to use
>> > README.md and look at the directions for building I tried running 'mvn
>> > compile' and received this output[2]. Also tried running the tests as
>> > detailed in the README.md and came away with this[3]
>> > Mind you this is a fresh machine that's never built Sentry before. (I
>> > also deleted my .m2/settings.xml to remove my local maven mirror and
>> > had the same problems.
>> >
>> > --David
>> >
>> > -1(binding)
>> >
>> > [1] http://incubator.apache.org/guides/releasemanagement.html#naming
>> > [2] https://paste.apache.org/9Dj5
>> > [3] https://paste.apache.org/7nnp
>> >
>> > On Wed, Apr 2, 2014 at 4:28 PM, Ramachandran, Karthik
>> > <[email protected]> wrote:
>> > > All,
>> > >
>> > > This is a release of Apache Sentry- version 1.3.0-incubating
>> > >
>> > > It fixes the following issues: http://s.apache.org/xXG
>> > >
>> > > Source files :
>> http://people.apache.org/~kramachandran/sentry-1.3.0-rc1/
>> > >
>> > > Tag to be voted on (release-1.3.0-rc1):
>> > >
>> >
>> https://git-wip-us.apache.org/repos/asf/incubator-sentry/repo?p=incubator-s
>> > > entry.git;a=log;h=refs/tags/release-1.3.0-rc1
>> > >
>> > > Sentry's KEYS containing the PGP key we used to sign the release:
>> > > https://people.apache.org/keys/group/sentry.asc
>> > >
>> > > Note that this is a source only release and we are voting on the source
>> > > (tag).
>> > >
>> > >
>> > > Vote will be open for 72 hours.
>> > >
>> > > [ ] +1 approve
>> > > [ ] +0 no opinion
>> > > [ ] -1 disapprove (and reason why)
>> > >
>> > >
>> > > Thanks,
>> > > Karthik
>> > >
>> > >
>> > >
>> > >
>> > > "This e-mail, and any attachments hereto, may contain information that
>> > is privileged, proprietary, confidential and/or exempt from disclosure
>> > under law and are intended only for the designated addressee(s). If you
>> are
>> > not the intended recipient of this message, or a person authorized to
>> > receive it on behalf of the intended recipient, you are hereby notified
>> > that you must not use, disseminate, copy in any form, or take any action
>> > based upon the email or information contained therein. If you have
>> received
>> > this email in error, please permanently and immediately delete it and any
>> > copies of it, including any attachments, and promptly notify the sender
>> at
>> > In-Q-Tel by reply e-mail, fax: 703-248-3001, or phone: 703-248-3000.
>> Thank
>> > you for your cooperation."
>> >
>>
>>
>>
>> --
>> Karthik Ramachandran
>> Mobile: 412-606-8981
>>
>
>
>
> --
> Sravya Tirukkovalur

Reply via email to