https://bugzilla.redhat.com/show_bug.cgi?id=1208738 --- Comment #6 from Taylor Braun-Jones <taylor@xxxxxxxxxxxxxxx> --- (In reply to Raphael Groner from comment #4) > Thanks for the fixes. So let's look deeper into the project details. > > (In reply to Taylor Braun-Jones from comment #3) > > (In reply to Raphael Groner from comment #2) > > > * MUST: Remove the Group: line. This tag is deprecated since Fedora 17. > > > https://fedoraproject.org/wiki/ > > > How_to_create_an_RPM_package#SPEC_file_overview > > > > So should I remove this even if I'm targeting EPEL6? > > Not sure for what it is needed at all. Neither am I. Removed. > > > * SHOULD: Why do you package the cmake and tcl files? Those look like source > > > or general development files to me. Shouldn't they be placed into a -devel > > > subpackage? Maybe do not include them at all (if in doubt). > > > > %{_libdir}/%{name}/*.cmake > > > > %{_libdir}/%{name}/*.cmake.in > > > … > > > > %{_libdir}/%{name}/rules/*.tcl > > > > %{_libdir}/%{name}/transformations/*.tcl > > > > These provide the CMake macros needed to integrate automatic vera++ checks > > into a CMake-based project. See "Running Vera++ as a test with CMake" and > > "Running Vera++ during the build with CMake" here: > > > > https://bitbucket.org/verateam/vera/wiki/Running > > * Okay, vera++ can be used as a standalone executable besides a full > integration into another project at build time. Please put those *.cmake* > files into an own (sub-)package named vera++-devel or cmake-vera++ because > they are not needed for the minimal use case. Put the main package as a > Requires. For the tcl files, please see my next comment as this seems to be > another special case. > https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages Okay, cmake files moved into devel subpackage > * Further, you would have to comply with tcl guideline in particular, > package especially the *.tcl files into tcl-vera++ as the special guideline > tells. Also pay attention to the other requirements there. > https://fedoraproject.org/wiki/Packaging:Tcl > > -- > Some other new things I see as general review points by now: > > * There is some ctest stuff provided in the upstream source. You could use > that for a %check section in your spec file. > https://bitbucket.org/verateam/vera/src/ > 30f9235797e5e6c34fda48bdaa05af1c0e558dd8/vera.ctest?at=master > https://fedoraproject.org/wiki/Packaging:Cmake#Specfile_Usage This is what I have in the current .spec file posted for review: %check cd build # Skip test that fails when run inside a mock chroot. See: # https://bitbucket.org/verateam/vera/issue/73 ctest --output-on-failure --exclude-regex RuleDUMP Is that what you are referring to? > * For EPEL, you have to use cmake28 as for RHEL instead of just cmake. > Please adjust the contitional. So you want something like this: %if (0%{?rhel} == 6) || (0%{?epel} == 6) Is that right? If so, I can certainly do that, but I'm not sure what the point of the extra conditional is. Under what scenario would check for (0%{?rhel} == 6) not be sufficient? > * Please put the generated files from the doc subfolders also in the package > (or a %{name}-doc named and noarch subpackage if the content is more than > 1MB in size), you could do this easily by help from the %doc macro. > https://fedoraproject.org/wiki/Packaging:Guidelines#Documentation The only doc file that gets generated is vera++.html which is only 59K. > * Plugins (lua, python, tcl) could also get their own subpackage named as > %{name}-plugins. But this is SHOULD only if it saves some significant space > in the main package (>1MB). > https://bitbucket.org/verateam/vera/src/ > 30f9235797e5e6c34fda48bdaa05af1c0e558dd8/src/plugins/?at=master > https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package > https://fedoraproject.org/wiki/How_to_create_an_RPM_package#Subpackages So even the .tcl files would go in this %{name}-plugins package? I like that approach best because even though the sum of all the .py, tcl, and .lua files is less than 200K currently, I expect that it will continue to grow since python and lua support is a brand new feature. And I think grouping all of these files into a %{name}-plugins seems more sensible and discoverable than putting some into a tcl-vera++ package. > * There's also a manpage generated that you should include into the package. Isn't that covered by the following line? %{_datadir}/man/man1/%{name}.1* -- 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