Biting the bullet?

[Matthew Miller]

On Tue, May 12, 2015 at 09:22:21AM -0500, Ranjan Maitra wrote:
> http://ur1.ca/kckv7 -> http://paste.fedoraproject.org/221087/14400891

Some comments inline; these will save you some time with the package
review.

>  Name:           stapler

I think pdf-stapler is a fine choice here, or pdfstapler.
python-stapler might also be okay.

>  Version:        master

Don't use "master" as the version. This is 0.3.0, according to the
setup.py and readme, but there's no corresponding release at
<https://github.com/hellerbarde/stapler/releases>. You might ask
upstream to _make_ one — that's the easiest way. Failing that, you
should treat this as a snapshot package — see guidelines
<https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages>


>  Release:        %{?dist}

There should be at least integer here, before the %{?dist}. It might
also include snapshot info — see again the guidelines above.

>  Summary:        Python-based PDF toolkit alternative to obsoleted pdftk

I'd suggest having this be a descriptive of what it is, rather than
what it is an alternative to. Something like:

Summary: tool for manipulating PDF documents from the command line

>  Group:          Applications/System

This tag is no longer used and should be omitted, unless you're going
to build for EPEL5.

>  License:        BSD
>  URL:            https://github.com/hellerbarde/stapler
>  Source0:        https://github.com/hellerbarde/stapler/archive/master.zip

See note on the source URL in my other message; in short, follow
https://fedoraproject.org/wiki/Packaging:SourceURL#Github

>  BuildRoot:  %{_tmppath}/%{name}-%{version}%{release}-root-%(%{__id_u} -n)

Another tag that's no longer needed.

>  BuildArch:      noarch
>  BuildRequires:  python-devel,PyPDF2,python-setuptools

pick one (or both) of python2-devel or python3-devel. Ideally, if it
works with python3, use that, since there's a plan to migrate all of
Fedora to Python 3 as default.

>  Requires:       PyPDF2


[...]

>  Like pdftk, stapler is a command-line tool. If you would like to add
>  a GUI, compile it into a binary for your favorite platform, or
>  contribute anything else, feel free to fork and send me a pull
>  request.

"Me" is unclear. Probably just remove that sentence.


>  %prep
>  %setup -q

as noted, probably '%setup -q -n stapler-%{version}' (I forgot the
version macro in my last message; sorry!)

[...]

>  %install
>  rm -rf $RPM_BUILD_ROOT

This rm line isn't necessary anymore. (Again, except for old epel)


> #desktop-file-validate
> # %{buildroot}/%{_datadir}/applications/%{name}.desktop
> #%find_lang %{name}

No desktop file or translations are included — just remove these line.


> install -m 755 -d $RPM_BUILD_ROOT/%{_sbindir}
> ln -s ../bin/eject $RPM_BUILD_ROOT/%{_sbindir}
 
Uh, I assume these are copied from an example package you were
following. :)

But, to accomplish the rename, what you want here is:

mv $RPM_BUILD_ROOT/%{_bindir}/stapler $RPM_BUILD_ROOT/%{_bindir}/%{name}

>  %clean
>  rm -rf $RPM_BUILD_ROOT

This whole section not necessary for recent Fedora.


>  %files
>  %defattr(-,root,root,-)
>  %doc CONTRIBUTORS LICENSE

Make that

%doc README.md CONTRIBUTORS TODO
%license LICENSE

>  %{_bindir}/stapler

Make it 

%{_bindir}/%{name}

>  %{_sbindir}/*

^ nothing in sbindir... remove that.

>  %{python2_sitelib}/*

Better to do 

%{python2_sitelib}/staplelib/


>  %changelog
>  * Mon Feb 09 2015 stat.maitra at inbox.com
>  - initial packaging of 0.3 version

See the changelog formats allowed at
http://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs

Hope this helps!


-- 
Matthew Miller
<mattdm at fedoraproject.org>
Fedora Project Leader