Spec URL: http://akurtakov.fedorapeople.org/sqljet.spec SRPM URL: http://akurtakov.fedorapeople.org/sqljet-1.0.1-1.fc12.src.rpm Description: SQLJet is an independent pure Java implementation of a popular SQLite database management system. SQLJet is a software library that provides API that enables Java application to read and modify SQLite databases.
0.) The -javadoc subpackage should requires jpackage-utils ...that owns %_javadocdir 1.) Does not build you probably meant to add junit4 to the classpath 2.) You don't use macros consistently please use either $RPM_BUILD_ROOT or %{buildroot} macro but not both The rest is just matter of clean style. Just for your considerations, probably nothing that would block the review: You can do this in one shot, without calling rm: -find -name '*.class' -exec rm -f '{}' \; -find -name '*.jar' -exec rm -f '{}' \; +find \( -name '*.class' -o -name '*.jar' \) -delete
3.) You do not run the test suite Please do so, ideally in %check section. Seems like you'd need to add a dependency on hamcrest. I'd also suggest replacing Summary with something more descriptive, (i.e. adding "database library" at the end or something like that). The rest looks well * Named and versioned in accordance with guidelines * License ok, license tag correct, license present in package documentation * spec file clean and legible * filelist sane
(In reply to comment #1) > 0.) The -javadoc subpackage should requires jpackage-utils > ...that owns %_javadocdir Fixed. > > 1.) Does not build > you probably meant to add junit4 to the classpath Fixed. > > 2.) You don't use macros consistently > please use either $RPM_BUILD_ROOT or %{buildroot} macro but not both Fixed. > > The rest is just matter of clean style. Just for your considerations, probably > nothing that would block the review: > > You can do this in one shot, without calling rm: > -find -name '*.class' -exec rm -f '{}' \; > -find -name '*.jar' -exec rm -f '{}' \; > +find \( -name '*.class' -o -name '*.jar' \) -delete Fixed. (In reply to comment #2) > 3.) You do not run the test suite > Please do so, ideally in %check section. Seems like you'd need to add a > dependency on hamcrest. Test suite is failing for me and upstream build is a nightmare. I've submitted a few issues about that but no response yet. Hopefully this is not a blocker. > > I'd also suggest replacing Summary with something more descriptive, (i.e. > adding "database library" at the end or something like that). Hmm, I'm not sure I understand what you want. I picked the first paragraph of sqljet.com assuming upstream devs will describe their work best. > > The rest looks well > * Named and versioned in accordance with guidelines > * License ok, license tag correct, license present in package documentation > * spec file clean and legible > * filelist sane New package: Spec URL: http://akurtakov.fedorapeople.org/sqljet.spec SRPM URL: http://akurtakov.fedorapeople.org/sqljet-1.0.1-2.fc12.src.rpm Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1837449
Thanks for the fixes. (In reply to comment #3) > (In reply to comment #2) > > 3.) You do not run the test suite > > Please do so, ideally in %check section. Seems like you'd need to add a > > dependency on hamcrest. > Test suite is failing for me and upstream build is a nightmare. I've submitted > a few issues about that but no response yet. Hopefully this is not a blocker. Sounds fair. > > I'd also suggest replacing Summary with something more descriptive, (i.e. > > adding "database library" at the end or something like that). > Hmm, I'm not sure I understand what you want. I picked the first paragraph of > sqljet.com assuming upstream devs will describe their work best. That was just a suggestion, feel free to keep your Summary if you feel it's better; definitely not a blocker or anything. APPROVED.
Thanks for the review. New Package CVS Request ======================= Package Name: sqljet Short Description: Pure Java SQLite Owners: akurtakov Branches: InitialCC:
cvs done.
http://koji.fedoraproject.org/koji/buildinfo?buildID=144190