[Bug 985051] Review Request: vcprompt - efficient program to print VCS info on your prompt

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

 



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



--- Comment #5 from Michael Schwendt <bugs.michael@xxxxxxx> ---
> PREFIX=/usr has to be replaced with PREFIX=%{prefix}.

Sorry for disagreeing here, but the guidelines are more lax nowadays:

  https://fedoraproject.org/wiki/Packaging:Guidelines#Macros

A hardcoded /usr for this tiny package would not be a blocker. Since the
package has been changed already, that's good. One can actually rebuild it with
"--define _prefix /foo", but the benefit is small.

[...]

> Summary:        An efficient program to print VCS information on your prompt

Even more consise, IMO:

  Summary: Print VCS information on your prompt


> Source0:        http://hg.gerg.ca/vcprompt/archive/%{version}.tar.gz

It would be great, if upstream could add the name "vcprompt" to the source
tarball. Currently, it's just "1.1.tar.gz", which may be considered enough as
part of a full URL, but on local storage it's too short and confusing.


> make %{?_smp_mflags}

https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

This "make" invocation in %build ought to set PREFIX=%{_prefix} and
MANDIR=%{_mandir}/man1, too, just in case the build process substitutes those
values into any built file (such as inline help or documentation). That's a
general hint for any package. For many packages there won't be any issues, but
there are exceptions.


> $ rpmls -p vcprompt-1.1-1.fc20.x86_64.rpm 
> -rwxr-xr-x  /usr/bin/vcprompt
> -rwxr-xr-x  /usr/man/man1/vcprompt.1.gz
> drwxr-xr-x  /usr/share/doc/vcprompt
> -rw-r--r--  /usr/share/doc/vcprompt/README.txt

The manual page should not be executable.

The path for the manual page is wrong, but only because the src.rpm includes an
old spec file that's missing some fixes.

If you keep the lines with "Spec URL:" and "SRPM URL:" in this ticket
up-to-date, you could point the fedora-review tool at this ticket:
fedora-review -b 985051

Running rpmlint (or "rpmlint -i …") on the src.rpm and all built rpms can be
helpful and is a requirement by the review guidelines, too:

  vcprompt.x86_64: W: spurious-executable-perm /usr/man/man1/vcprompt.1.gz
  vcprompt.x86_64: W: manual-page-warning /usr/man/man1/vcprompt.1.gz 284: a
space character is not allowed in an escape name
  vcprompt.x86_64: W: non-standard-dir-in-usr man


> Makefile

It includes a "check" target, which executes fine here, but skips some tests.
Do you think all tests in that test-suite is suiteable for a %check section?
Then add

  %check
  make check

after the %install section. It may require additional BuildRequires for some of
the tests. If any of the tests require network access, that won't be possible
in the Fedora Build System, however.

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