Hi David, thank you very much for your feedback, greatly appreciated! You've raised couple of really good points.
The rat issues is interesting - indeed we are not setting the excludeSubProjects to false but at the same time rat is checking all subprojects. I've just tried to put incorrectly licensed file into one of the sub modules and it has been correctly detected, so I'm assuming that this one should be covered. Looking at the rest of the feedback it seems that nothing is a release blocker per say, so I'm wondering if you are fine with us moving forward with the release? Jarcec On Thu, Apr 03, 2014 at 11:25:08PM -0400, David Nalley wrote: > 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
signature.asc
Description: Digital signature
