https://bugzilla.redhat.com/show_bug.cgi?id=1121601 David Nichols <david@xxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@xxxxxxxx --- Comment #1 from David Nichols <david@xxxxxxxx> --- HI, this is an informal review fedora-review was not able to find the following files: INFO: No upstream for (Source3): rt.conf.in INFO: No upstream for (Source1): README.tests INFO: No upstream for (Source4): README.fedora.in INFO: No upstream for (Source5): rt.logrotate.in so I was not able to check them with the build (note that there is no Source2). and here is the annotated output of fedora-review: Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ==== MUST items ===== Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated". 202 files have unknown license. Detailed output of licensecheck in /export/home/dnichols/fr/review-rt/licensecheck.txt [x]: License file installed when any subpackage combination is installed. the rt-tests and perl-RT-Test packages don't contain the license file, but they are test packages, and since this is based on the already-approved rt3 package, I assume that this is OK. [x]: Package must own all directories that it creates. Note: Directories without known owners: /etc/logrotate.d, /usr/libexec/perl5-tests these are OK: https://fedoraproject.org/wiki/User:Johannbg/Packaging/LogFiles also the perl5-tests directory is a shared directory designed using macros in the spec file [ ]: Package does not own files or directories owned by other packages. Note: Dirs in package are owned also by: /usr/share/perl5/vendor_perl/RT/Interface/Web(rt3), /usr/share/perl5/vendor_perl/RT/Shredder/Plugin/Base(rt3), /usr/share/perl5/vendor_perl/RT/Interface/Web/QueryBuilder(rt3), /usr/share/perl5/vendor_perl/RT/Condition(rt3), /usr/share/perl5/vendor_perl/RT/Crypt(rt3), /usr/share/perl5/vendor_perl/RT/Interface/Email(perl-RT-Extension- CommandByMail, rt3), /usr/share/perl5/vendor_perl/RT/Approval/Rule(rt3), /usr/share/perl5/vendor_perl/RT/Graph(rt3), /usr/share/perl5/vendor_perl/RT/Interface(perl-RT-Extension- CommandByMail, rt3), /usr/share/perl5/vendor_perl/RT/Approval(rt3), /usr/share/perl5/vendor_perl/RT/Interface/Email/Auth(rt3), /usr/share/perl5/vendor_perl/RT/Shredder/Plugin(rt3), /usr/share/perl5/vendor_perl/RT/Search(rt3), /usr/share/perl5/vendor_perl/RT/I18N(rt3), /usr/share/perl5/vendor_perl/RT/Shredder(rt3), /usr/share/perl5/vendor_perl/RT/CustomFieldValues(rt3), /usr/share/perl5/vendor_perl/RT/Report(rt3), /usr/share/perl5/vendor_perl/RT/URI(rt3), /usr/share/perl5/vendor_perl/RT/Report/Tickets(rt3), /usr/share/perl5/vendor_perl/RT/Action(rt3), /usr/share/perl5/vendor_perl/RT(perl-RT-Client-REST, perl-RT-Test, perl- RT-Extension-CommandByMail, perl-RT-Authen-ExternalAuth, rt3) rt replaces rt3, so these are OK [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [ ]: If the package is a rename of another package, proper Obsoletes and Provides are present. rt.spec: Obsoletes: rt3 < %{version}-%{release} I understand that it's better to hardcode the version in the Obsoletes line - to quote a more experienced fedora reviewer (Michael Schwendt): "typically one hardcodes a specific maximum version-release in the Obsoletes tag to be really accurate and not obsolete more than necessary (e.g. if %version increments, the Obsoletes tag would adjust and also obsolete newer versions than what had been specified originally)." Therefore maybe this should be Obsoletes: rt3 < 4.0.21 [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Package is not known to require an ExcludeArch tag. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 931840 bytes in 39 files. [x]: Package complies to the Packaging Guidelines I base my opinion to the previous successful package review for rt3 for this and other issues: https://bugzilla.redhat.com/show_bug.cgi?id=169247 Perl: [x]: Package contains the mandatory BuildRequires and Requires:. I'm not sure why fedora-review flags this issue. According to: http://fedoraproject.org/wiki/Packaging:Perl#Testing_and_Test_Suites the spec file should be OK, and also since it's based on rt3, I assume it's OK. ===== SHOULD items ===== Generic: [!]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. Note: Incorrect Requires : /usr/share/fonts/google-droid/DroidSans.ttf, /usr/share/fonts/google-droid/DroidSansFallback.ttf See: http://fedoraproject.org/wiki/Packaging/Guidelines#FileDeps I guess I don't understand this, because the rt soure package provides these files, so I don't understand why they are listed as Requires and BuildRequires as well. for example, I could not build the RPMs due to the following error: dnichols@xbox:~/fr$ rpmbuild --recompile rt-4.0.21-1.fc22.src.rpm Installing rt-4.0.21-1.fc22.src.rpm /usr/share/fonts/google-droid/DroidSansFallback.ttf is needed by rt-4.0.21-1.fc20.noarch /usr/share/fonts/google-droid/DroidSans.ttf is needed by rt-4.0.21-1.fc20.noarch [ ]: Avoid bundling fonts in non-fonts packages. Note: Package contains font files If these are useful fonts, then maybe they should be packaged with other fedora fonts or in a separate font package? [x]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. COPYING (GPLv2+) is included, not sure why fedora-review doesn't see it. [ ]: Final provides and requires are sane (see attachments). I assume they are but was unable to test the build. [x]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in rt-mailgate I assume this is OK since it's Perl-based. [ ]: Package functions as described. I was not able to test the build due to issues reported above. [x]: Latest version is packaged. I get your logic for packaging an older build, and I trust your judgement :). [x]: Package does not include license text files separate from upstream. [ ]: Patches link to upstream bugs/comments/lists or are otherwise justified. your patch lines do not have any comments: http://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment I'm not sure how important this is because other more experienced packagers have also ignored this directive. Anyway the patch names are self-explanatory. As a newbie I would comment these lines, but as an experienced packager, I assume you know what you're doing :) [x]: Scriptlets must be sane, if used. [x]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [ ]: Package should compile and build into binary rpms on all supported architectures. I was not able to test this due to issues reported above. [ ]: %check is present and all tests pass. I was not able to test this due to issues reported above. [x]: Packages should try to preserve timestamps of original installed files. perms are generally set manually, the testsuite does not try to preserve permissions, but I guess that's OK -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review