[Bug 703322] Review Request: tpp - text presentation program

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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

Golo Fuchert <packages at golotop.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |packages at golotop.de

--- Comment #4 from Golo Fuchert <packages at golotop.de> 2011-05-12 16:25:17 EDT ---
Hello Jesus,

yes, it is ok to use $RPM_BUILD_ROOT, just don't mix them.
Furthermore, copy-paste is totally fine when it comes to packaging, however,
you should really use a better template next time. ;-)
Here are a few things that should be fixed before a review.

- Development/Languages is maybe not the best choice for this package. I mean 
  it is more like an application, isn't it?
- There is a little confusion about the license. The tpp source file states 
  GPLv2, while the README states GPLv2+. Upstream should be approached for 
  clarification. If they don't react, GPLv2 is the correct choice.
- It is not wise to hard-code the name and version in the Source0 tag, when you 
  can use %{name} and %{version} instead and save you some work in the future.
- Just a little typo in the description: [...] _a_ ncurses-based [...] 
- The %install section is a bit ugly. You create a few directories just to 
  extract the data with the more than simple Makefile, which you even have to 
  "fedorarize" before usage. For that you create the unneeded dir 
  $RPM_BUILD_ROOT%{_datadir}/%{name} (and you do it twice, just to be sure it 
  really worked) and you create the docdir yourself, which you don't have to,
  when packaging the docs appropriately.
  My suggestion: Why not just throw the Makefile away, copy tpp to 
  $RPM_BUILD_ROOT/%{_bindir} (using install -p or cp -p), add the vim and emacs 
  files to the directories where they can be useful (have a look at the package 
  vim-filesystem, which the package could require and at [1] for emacs).
  Finally, add the docfiles by using %doc and then just list the appropriate
  files and subdirs, the docdir is created automatically then. Do not, however,
  add the manpage as %doc, this is also done automatically. Use 
  %{_mandir}/man1/tpp.1.*
  instead. (the * makes sure that the manpage is packaged correctly, even when
  the compression method would be changed or even dropped). 
- Why do you repeat your mail address in the changelog entries? I am referring 
  to the second occurrence in the actual descriptions, for example:

* Wed May 11 2011 jesus m. rodriguez <jesusr at redhat.com> 1.3.1-4
# why is the e-mail address repeated here?
- don't use RPM_BUILD_ROOT and buildroot --> (jesusr at redhat.com) <--

- You do know rpmlint, right? It reveals (among some false positives):
rpmlint tpp-1.3.1-4.fc14.noarch.rpm                                        
tpp.noarch: W: spelling-error Summary(en_US) ncurses -> nurses, curses, n
curses                                                
tpp.noarch: W: summary-not-capitalized C ncurses-based presentation tool  
tpp.noarch: W: spelling-error %description -l en_US ncurses -> nurses, curses,
n curses                                                                      
tpp.noarch: W: spelling-error %description -l en_US framebuffer -> frame
buffer, frame-buffer, framework
tpp.noarch: E: incorrect-fsf-address /usr/share/doc/tpp-1.3.1/COPYING
tpp.noarch: W: file-not-utf8 /usr/share/doc/tpp-1.3.1/examples/ac-am.tpp
tpp.noarch: W: file-not-utf8 /usr/share/doc/tpp-1.3.1/examples/slidein.tpp
tpp.noarch: W: file-not-utf8 /usr/share/doc/tpp-1.3.1/examples/tpp-features.tpp
tpp.noarch: W: file-not-utf8 /usr/share/doc/tpp-1.3.1/examples/test.tpp      
tpp.noarch: W: file-not-utf8 /usr/share/doc/tpp-1.3.1/examples/bold.tpp
tpp.noarch: W: file-not-utf8 /usr/share/doc/tpp-1.3.1/examples/huge.tpp
tpp.noarch: W: file-not-utf8
/usr/share/doc/tpp-1.3.1/examples/debian-packaging.tpp
1 packages and 0 specfiles checked; 1 errors, 11 warnings.

  Concerning the utf8 encoding warnings refer to [2]
  The FSF address error means that an outdated address of the FSF is found in 
  the license file. This should be fixed and probably even be reported
upstream.

---

This may seem like a lot, but must issues can by fixed quite quickly. And when
you are finished, we do the review and you have a better template next time. :)

---

[1]
http://fedoraproject.org/wiki/Packaging:Emacs#Case_II:_Package_also_provides_auxiliary_.28X.29Emacs_files
[2] http://fedoraproject.org/wiki/Common_Rpmlint_issues#file-not-utf8

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.