[Bug 1208738] Review Request: vera++ - A tool for verification, analysis and transformation of C++ source code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1208738



--- Comment #3 from Taylor Braun-Jones <taylor@xxxxxxxxxxxxxxx> ---
(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?

> * SHOULD: You can use the %{url} macro to shorten the links:
> > URL:            https://bitbucket.org/verateam/vera/wiki/Home
> > Source0:        https://bitbucket.org/verateam/vera/downloads/%{name}-%{version}.tar.gz
> URL:            https://bitbucket.org/verateam/vera
> Source0:        %{url}/downloads/%{name}-%{version}.tar.gz

Fixed, thanks.

> * MUST: Where do you have the patch from? Please add a comment. You could
> inform upstream and use a link into the upstream tracker.
> > Patch0:         0001-add_LIB_SUFFIX_to_lib_install_rules.patch

Fixed

> * SHOULD: Please add popd at the end of %build to identicate where your
> pushd environment ends.
> > %build
> > mkdir build
> > pushd build

Fixed

> * SHOULD: Move the following rm line from %install into separate %clean
> section and better use the more commonly used %{buildroot} macro instead.
> Instructions to clean out the build root. Note that this section is now
> redundant in Fedora and is only necessary for EPEL.
> https://fedoraproject.org/wiki/
> How_to_create_an_RPM_package#SPEC_file_overview
> > %install
> > rm -rf $RPM_BUILD_ROOT
> %clean
> rm -rf %{buildroot}

Fixed

> * SHOULD: The README.txt file can go outside of the conditional to avoid
> further confusion.
> > %files
> > %if (0%{?rhel} == 06)
> > %doc README.txt LICENSE_1_0.txt
> > %else
> > %doc README.txt
> > %license LICENSE_1_0.txt
> > %endif
> %files
> %if (0%{?rhel} == 06)
> %doc LICENSE_1_0.txt
> %else
> %license LICENSE_1_0.txt
> %endif
> %doc README.txt

Good point, fixed.

> * SHOULD: Use the %{name] macro consistently. Also, there should not be an
> extension to any manpage cause rpmbuild can do the compression magically.
> For the documentation folder you can use the shorter %{_pkgdocdir} macro.
> > %{_bindir}/vera++
> …
> > %{_datadir}/man/man1/vera++.1.gz
> > %{_datadir}/%{name}/doc/vera++.html
> %{_bindir}/{name}
> …
> %{_datadir}/man/man1/%{name}.1*
> %{_pkgdocdir}/%{name}.html

Fixed for the man page, but %{_pkgdocdir} doesn't work on EPEL[67]

> * 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

> I could do also an official fedora-review run when my questions are answered.

Files are updated and ready for review. Thanks!

-- 
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





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]