Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=727670 --- Comment #2 from Omair Majid <omajid@xxxxxxxxxx> 2011-09-26 16:30:41 EDT --- (In reply to comment #1) > Hi, > I'm just learning fedora package review stuff. Please consider this an > *informal* review. I based this on a couple reviews I found for other > packages. Thanks; your review is appreciated! > simplevalidation.spec:14: W: mixed-use-of-spaces-and-tabs (spaces: line 14, > tab: line 1) > > Not an actual problem, but would be nice to be consistent. > Fixed. > simplevalidation.spec: W: invalid-url Source0: http://kenai.com/project > /simplevalidation/downloads/download/validation-src.zip HTTP Error 404: Not > Found I am not sure why this error is happening - the url works for me. > [!] Package consistently uses macros (no %{buildroot} and $RPM_BUILD_ROOT > mixing) > > Some use of $foo (variables) and some %{bar} macros, please pick one. > (see > http://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS) > > (NB: It's not entirely clear to me whether this is meant to apply to all use of > variables/macros, or *just* the specific variables noted on the wiki. If it is > just aimed at those specific variables, feel free to ignore this) > >From what I understand, it is only for %{buildroot} vs RPM_BUILD_ROOT and %{optflags} vs $RPM_OPT_FLAGS, not for general variables. But I have "fixed" it anyway. > [!] Package has BuildArch: noarch (if possible) > > This is pure java, no native code? Unless there is reason to produce > arch-specific packages, please specify noarch. > Whoops. My mistake. Fixed now. > === Final Notes, AKA Questions from the Noob === > I notice in the files section, you use a wildcard even though there is only the > single jarfile belonging to the package. I can't find any guidelines about > this, but it seems to me that what the specfile is doing here would be more > clear if the file was specified, much as it is in the install section. Is > there some guideline I have missed? > No, I am not aware of any such guideline. I don't think it makes too much of a difference either way. * may be a little more future-proof in case the jar gets renamed. But I have made the name explicit now. > One more nit: Extra newline amidst BuildRequires lines should probably be > removed. > Actually, I would rather keep this. It separates the packages required by the java packaging policy (jpackage-utils and java-devel) from the actual build-dependencies. > Hope this is helpful! It is. Thanks. Updated files: Spec URL: http://omajid.fedorapeople.org/simplevalidation/simplevalidation.spec SRPM URL: http://omajid.fedorapeople.org/simplevalidation/simplevalidation-0.4-1.fc15.src.rpm -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review