Bug 541589
Summary: | Review Request: sqljet - Pure Java SQLite | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Alexander Kurtakov <akurtako> |
Component: | Package Review | Assignee: | Lubomir Rintel <lkundrak> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, lkundrak, notting |
Target Milestone: | --- | Flags: | lkundrak:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2009-12-03 11:09:11 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Alexander Kurtakov
2009-11-26 12:34:24 UTC
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. |