[Bug 1234210] New: Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Bug ID: 1234210
           Summary: Review Request: pdf-stapler - tool for manipulating
                    PDF documents from the command line
           Product: Fedora
           Version: rawhide
         Component: Package Review
          Severity: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]

Spec URL: https://streaming.stat.iastate.edu/~stat580/fedora/pdf-stapler.spec
SRPM URL:
https://streaming.stat.iastate.edu/~stat580/fedora/pdf-stapler-0.3.0-1.fc22....
Description: PDFtk was written in Java and C++, and is natively compiled with
gcj. Sadly, it 
has been discontinued a few years ago and bitrot is setting in (e.g., it does
no
t compile easily on a number of platforms).

Philip Stark decided to look for an alternative and found pypdf, a PDF library
w
ritten in pure Python. He couldn't find a tool which actually used the library, 
so he started writing his own.

This version of stapler is Fred Wenzel's fork of the project, with a completely 
refactored source code, tests, and added functionality.

Like pdftk, stapler is a command-line tool. If you would like to add a GUI,
comp
ile it into a binary for your favorite platform, or contribute anything else,
fe
el free to fork and send Philip Stark a pull request.

Fedora Account System Username: maitra

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Zbigniew Jędrzejewski-Szmek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
           Assignee|[e-mail]    |[e-mail]
              Flags|                            |fedora-review?

--- Comment #1 from Zbigniew Jędrzejewski-Szmek  ---
%description should be wrapped to 72 columns. But please change
the text to something which describes what the package does (history
is not relevant).

Summary should start with a capital letter.

Remove %defattr.

There are some stale comments, remove them too.

No need to run sed in a loop, just pass all the file names to sed at once.
Also, are you sure that you want to encode pypdf version in the file? This
package will have to be updated whenever the pypdf package is updated to a new
version. Why not remove the pypdf version (s/pypdf == .*/pypdf/) ?

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #2 from Globe Trotter  ---
(In reply to Zbigniew Jędrzejewski-Szmek from comment #1)
> %description should be wrapped to 72 columns. But please change
> the text to something which describes what the package does (history
> is not relevant).

I have included a short description but also left a bit of the history as well
as the dependencyon PyPDF2. I think this will be helpful for future packagers.
> Summary should start with a capital letter.

done
> Remove %defattr.

done
> There are some stale comments, remove them too.

I presume that these means the commented-out codes? Done, but for commented out
Requires.
> No need to run sed in a loop, just pass all the file names to sed at once.

I am not sure how to do this, please advise if this is a serious issue.
> Also, are you sure that you want to encode pypdf version in the file? This
> package will have to be updated whenever the pypdf package is updated to a
> new version. Why not remove the pypdf version (s/pypdf == .*/pypdf/) ?

pyPdf is dead. The only update will be to PyPDF2. That is also my package which
is also under review. So, at this point, this is not particularly an issue?

New source rpms and spec files at:

Spec URL: https://streaming.stat.iastate.edu/~stat580/fedora/pdf-stapler.spec
SRPM URL:
https://streaming.stat.iastate.edu/~stat580/fedora/pdf-stapler-0.3.0-1.fc22....

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #3 from Zbigniew Jędrzejewski-Szmek  ---
(In reply to Globe Trotter from comment #2)
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #1)
> > No need to run sed in a loop, just pass all the file names to sed at once.
>
> I am not sure how to do this, please advise if this is a serious issue.

for f in setup.py stapler.egg-info/requires.txt ; do
    sed -i 's/pypdf == 1.12/pypdf == .*/g' $f
done

can be changed to

sed -i 's/pypdf == 1.12/pypdf == .*/g' setup.py stapler.egg-info/requires.txt

Nothing serious, just easier to read.
Actually You could just 'rm -rf stapler.egg-info', it would be recreated during
installation.
> > Also, are you sure that you want to encode pypdf version in the file? This
> > package will have to be updated whenever the pypdf package is updated to a
> > new version. Why not remove the pypdf version (s/pypdf == .*/pypdf/) ?
>
> pyPdf is dead. The only update will be to PyPDF2. That is also my package
> which is also under review. So, at this point, this is not particularly an
> issue?

OK.
> Spec URL: https://streaming.stat.iastate.edu/~stat580/fedora/pdf-stapler.spec
> SRPM URL:
> https://streaming.stat.iastate.edu/~stat580/fedora/pdf-stapler-0.3.0-1.fc22.
> src.rpm

Please install the license file using %license
[https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text].

Add /usr/lib/python2.7/site-packages/staplelib to %files.

pdf-stapler.noarch: W: incoherent-version-in-changelog [e-mail]
['0.3.0-1.fc23', '0.3.0-1']

Also, change %{__python} to %{__python2} (I'm assuming that python3 is not
supported)
[https://fedoraproject.org/wiki/Packaging:Python#Multiple_Python_Runtimes].

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #4 from Globe Trotter  ---
Sorry, I am new to this.
> for f in setup.py stapler.egg-info/requires.txt ; do
>     sed -i 's/pypdf == 1.12/pypdf == .*/g' $f
> done
>
> can be changed to
>
> sed -i 's/pypdf == 1.12/pypdf == .*/g' setup.py stapler.egg-info/requires.txt
>
> Nothing serious, just easier to read.
> Actually You could just 'rm -rf stapler.egg-info', it would be recreated
> during installation.

So, instead of the sed, replace with

rm -rf stapler.egg-info

(I tried this, and it "worked".)
> Please install the license file using %license
> [https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text].

I am sorry but how do I add the file?
> Add /usr/lib/python2.7/site-packages/staplelib to %files.

I did this but I now get warnings, eg:

warning: File listed twice:
/usr/lib/python2.7/site-packages/staplelib/__init__.py

....
> pdf-stapler.noarch: W: incoherent-version-in-changelog [e-mail]
> ['0.3.0-1.fc23', '0.3.0-1']

Not sure I understand this point.
> Also, change %{__python} to %{__python2} (I'm assuming that python3 is not
> supported)
> [https://fedoraproject.org/wiki/Packaging:Python#Multiple_Python_Runtimes].

At this point, however, pdf-stapler is moving to python3, in which case I will
also move it to python3. (I don't quite see the point of supporting both..)

Thanks again!

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #5 from Zbigniew Jędrzejewski-Szmek  ---
(In reply to Globe Trotter from comment #4)
> > Please install the license file using %license
> > [https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text].
>
> I am sorry but how do I add the file?
> > Add /usr/lib/python2.7/site-packages/staplelib to %files.

Use something like that for %files:

%files
%{_bindir}/%{name}
/usr/lib/python2.7/site-packages/stapler-%{version}-py2.7.egg-info
%{python2_sitelib}/staplelib
%license LICENSE

(This will include the directory .../staplelib and its contents.)
(It will also install the file LICENSE into %{_pkgdocdir}.)
> I did this but I now get warnings, eg:
>
> warning: File listed twice:
> /usr/lib/python2.7/site-packages/staplelib/__init__.py
>
> ....
> > pdf-stapler.noarch: W: incoherent-version-in-changelog [e-mail]
> > ['0.3.0-1.fc23', '0.3.0-1']
>
> Not sure I understand this point.

You are missing version information in the changelog. The line look should
look something like this:

* Fri May 22 2015 Name Lastname  - 0.3.0-1
> > Also, change %{__python} to %{__python2} (I'm assuming that python3 is not
> > supported)
> > [https://fedoraproject.org/wiki/Packaging:Python#Multiple_Python_Runtimes].
>
> At this point, however, pdf-stapler is moving to python3, in which case I
> will also move it to python3. (I don't quite see the point of supporting
> both..)

That's great. The distribution as a whole is trying to move to python3 as much
as possible
[https://fedoraproject.org/wiki/Changes/Python_3_as_Default]. So if you can can
switch pdf-stapler to use python3 that would be great. But independently of the
switch, whatever version is used by the application, it is supposed to be
clearly marked. So if it is using python2, then use %{__python2} (which is
/usr/bin/python2), otherwise use %{__python3} (which is /usr/bin/python3). This
is done in preparation for possibly switching /usr/bin/python to point at
python3 instead of python2 at some point.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Michael Schwendt (Fedora Packager Sponsors Group)  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |177841 (FE-NEEDSPONSOR)

--- Comment #6 from Michael Schwendt (Fedora Packager Sponsors Group)  ---
Fedora Account System (FAS) tells you're not in the packager group yet.

Seems you've missed step 3 from 2.1
https://fedoraproject.org/wiki/Package_Review_Process#Contributor process.

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841
[Bug 177841] Tracker: Review requests from new Fedora packagers who need a
sponsor

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #7 from Zbigniew Jędrzejewski-Szmek  ---
Ooops, I didn't check that. In that case I'm happy to work with you on bringing
the package to follow the guidelines, but the final decision about the review
will belong to the sponsor.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Zbigniew Jędrzejewski-Szmek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
           Assignee|[e-mail]           |[e-mail]

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #8 from Globe Trotter  ---
How does someone block the FE-NEEDSPONSOR tracking bug?

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #9 from Zbigniew Jędrzejewski-Szmek  ---
(In reply to Globe Trotter from comment #8)
> How does someone block the FE-NEEDSPONSOR tracking bug?

I already added that. You can see "Blocks:     FE-NEEDSPONSOR" near the top of
the page.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #10 from Globe Trotter  ---
Response to Comment 3:

The author has finally added the License file to upstream.

However, now he has also explicitly required PyPDF2 instead of PyPdf which is
also submitted to packaging:

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

How does one proceed to submit a package for review which also depends on
another that has also been submitted for review and no one has picked it yet?
(The original PyPDF2 was submitted at the same time as this one:
https://bugzilla.redhat.com/show_bug.cgi?id=1234208)

Many thanks!

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #11 from Zbigniew Jędrzejewski-Szmek  ---
(In reply to Globe Trotter from comment #10)
> Response to Comment 3:
>
> The author has finally added the License file to upstream.
>
> However, now he has also explicitly required PyPDF2 instead of PyPdf which
> is also submitted to packaging:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1262470

OK. I'll try to review that.
> How does one proceed to submit a package for review which also depends on
> another that has also been submitted for review and no one has picked it
> yet? (The original PyPDF2 was submitted at the same time as this one:
> https://bugzilla.redhat.com/show_bug.cgi?id=1234208)

By adding the other bug to "Depends On". Both reviews can proceed in parallel,
the only caveat is that if this review is finished before the other one, you
will not be able to actually build the package officially in koji.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Matthias Runge  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #12 from Matthias Runge  ---
Could you please put the spec file and srpm back? Currently, it's a 404 for me.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #13 from [e-mail] ---
Upstream here.

There is a separate setup.py file in the git repository, if the software has to
be packaged with the old pyPdf as a dependency. Be aware that it's not as well
tested.

If there is anything I can help with, let me know.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #14 from Globe Trotter  ---
(In reply to Matthias Runge from comment #12)
> Could you please put the spec file and srpm back? Currently, it's a 404 for
> me.

New files uploaded here:

SRPM: http://maitra.public.iastate.edu/Fedora/pdf-stapler-0.3.3-1.fc22.src.rpm
SPEC: http://maitra.public.iastate.edu/Fedora/pdf-stapler.spec

Both use python-PyPDF2.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Matthias Runge  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends On|                            |1262470

--- Comment #15 from Matthias Runge  ---
Globe Trotter,

could you please:
- remove code in comments (like in %prep section)
- you should require pyPDF2 like in setup, requiring packages under review is
just fine. I added that as depending review.
- there are tests in github repo. it would be great to execute them during
package build. It gives you a bit more feedback, if your build works (or not)
- latest version is 0.3.3, please update; I see, you forgot to add a line in
changelog
- *I would* separate BuildRequires:  python2-devel,python-setuptools 
into two lines:
BuildRequires:  python2-devel
BuildRequires:  python-setuptools

- P Stark, Globe Trotter, Fedora is moving to be python3 by default. It would
be great to provide a python3 (sub) package as well. P Stark, do you support
python3 here, too?
- PyPDF2 apparently supports python3
- please remove comments from %files section

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1262470
[Bug 1262470] Review Request: python-PyPDF2 - A pure Python library built
as a PDF toolkit

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #16 from Pete Travis  ---
Globe Trotter, as a side note, your rhbz account doesn't seem to have anything
in common with your FAS account.  The different email account *will* cause
problems, you should change one or the other so they are the same.  A real name
in both also helps us know who we're working with.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #17 from P Stark  ---
I have a Python 3 porting branch. It's almost done, but it needs some more
tests and my time is severely limited currently. I'd say the ETA for Python 3
support is a conservative 2-3 months.

PyPDF2 supports Python 3, but the old pyPdf doesn't. I will be happy to drop
support for the old pyPdf completely! :D

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #18 from Globe Trotter  ---
Done and new files uploaded at the same site.
> you should require pyPDF2 like in setup, requiring packages under review
>
> is just fine. I added that as depending review.

not completely sure how/what to do here. I put this in as a comment, but I
thought you did not want that.
> there are tests in github repo. it would be great to execute them during
>
> package build. It gives you a bit more feedback, if your build works (or not)

How do I do this? Sorry, I am not a python programmer and this is my first
package anyway.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #19 from Matthias Runge  ---
> > there are tests in github repo. it would be great to execute them during
> >
> > package build. It gives you a bit more feedback, if your build works (or not)
>
> How do I do this? Sorry, I am not a python programmer and this is my first
> package anyway.

you could add something like:

%check
%{__python} setup.py test

to execute tests.

Be careful: build systems are not connected to the net, thus downloading from
pypi doesn't work.

you probably need to sed out requirements:

sed -i 's|"PyPDF2>=1.24"||' setup.py

That has to be done in %prep.

Requirements are handled in rpm anyways, there's no much value in keeping them
tied in python.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #20 from Globe Trotter  ---
New files uploaded as per suggested lines. Please check and see if there is
anything else to be fixed.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #21 from Globe Trotter  ---
Just to clarify, new files have been uploaded here:

SRPM: http://maitra.public.iastate.edu/Fedora/pdf-stapler-0.3.3-1.fc22.src.rpm
SPEC: http://maitra.public.iastate.edu/Fedora/pdf-stapler.spec

Both use python-PyPDF2.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #22 from Matthias Runge  ---
python-PyPDF2 was approved and imported into repositories

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1234210
Bug 1234210 depends on bug 1262470, which changed state.

Bug 1262470 Summary: Review Request: python-PyPDF2 - A pure Python library built as a PDF toolkit
https://bugzilla.redhat.com/show_bug.cgi?id=1262470

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ON_QA                       |CLOSED
         Resolution|---                         |ERRATA

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Michael Schwendt  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |

--- Comment #23 from Michael Schwendt  ---
Nobody has reset the fedora-review flag for months. Ticket has not been visible
in the needsponsor queue due to that, which is not the only reason for no
progress, however.

Btw:
> %{python2_sitelib}/staplelib/*

https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Owner...
https://fedoraproject.org/wiki/Packaging:UnownedDirectories

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #24 from Ranjan Maitra  ---
New files, corrected, have been uploaded here:

SRPM: http://maitra.public.iastate.edu/Fedora/pdf-stapler-0.3.3-1.fc23.src.rpm
SPEC: http://maitra.public.iastate.edu/Fedora/pdf-stapler.spec

Both use python-PyPDF2.

rpmlint only gives some mistaken typographical errors.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #25 from Ranjan Maitra  ---
Please let me know what additionally is needed to get this through to
packaging.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #26 from Ranjan Maitra  ---
Fedora Account System Username: maitra

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #27 from Upstream Release Monitoring  ---
maitra's scratch build of pdf-stapler-0.3.3-1.fc23.src.rpm for f23 completed
http://koji.fedoraproject.org/koji/taskinfo?taskID=12027071

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #28 from Zbigniew Jędrzejewski-Szmek  ---
(In reply to Ranjan Maitra from comment #25)
> Please let me know what additionally is needed to get this through to
> packaging.

In Fedora new packagers are required to do some extra work to show that they
know the packaging rules and various procedures related to packaging. See
comment #6.

To get the package into Fedora, you need both things: become a packager, and
make the package pass review. This second part is almost done, but to do the
first, please see the docs from comment #6. Usually, doing informal reviews of
a few packages from http://fedoraproject.org/PackageReviewStatus/NEW.html or
http://fedoraproject.org/PackageReviewStatus/REVIEW.html is a welcome step
(start with the recent tickets, they are at the bottom in both lists).

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #29 from Upstream Release Monitoring  ---
maitra's scratch build of pdf-stapler-0.3.3-1.fc23.src.rpm for rawhide
completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12027171

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #30 from Ranjan Maitra  ---
New builds uploaded to account for some minor issues:

Documentation now includes README.rst file and PKG-INFO, and License is
BSD-style rather than BSD as per upstream.

New files uploaded:

SRPM: http://maitra.public.iastate.edu/Fedora/pdf-stapler-0.3.3-2.fc23.src.rpm
SPEC: http://maitra.public.iastate.edu/Fedora/pdf-stapler.spec

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Parag AN(पराग)  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #31 from Parag AN(पराग)  ---
I just checked above spec file link. Here are some suggestions

1) If you are packaging python2 library or tools then your spec file should use
%{__python2} macros only. See
https://fedoraproject.org/wiki/Packaging:Python#Macros

2) Its always a good practice to increase the release number when you provide
updated package here. That will help what has changed since your last package
update to new package update.

3) The recent changelog entry looks longer than 80 characters.

4) Good to add comment just above the mv command line in %install that 
# Fedora already have stapler package that is
# why this package is named as pdf-stapler

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #32 from Ranjan Maitra  ---
(In reply to Parag AN(पराग) from comment #31)
> I just checked above spec file link. Here are some suggestions
> If you are packaging python2 library or tools then your spec file should
>
> use %{__python2} macros only. See
> https://fedoraproject.org/wiki/Packaging:Python#Macros
> Its always a good practice to increase the release number when you
>
> provide updated package here. That will help what has changed since your
> last package update to new package update.
>
> The recent changelog entry looks longer than 80 characters.
>
> Good to add comment just above the mv command line in %install that
>
> # Fedora already have stapler package that is
> # why this package is named as pdf-stapler

Done.

New files uploaded:

SRPM: http://maitra.public.iastate.edu/Fedora/pdf-stapler-0.3.3-3.fc23.src.rpm
SPEC: http://maitra.public.iastate.edu/Fedora/pdf-stapler.spec

(

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Zbigniew Jędrzejewski-Szmek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
             Blocks|177841 (FE-NEEDSPONSOR)     |
           Assignee|[e-mail]    |[e-mail]
              Flags|                            |fedora-review?

--- Comment #33 from Zbigniew Jędrzejewski-Szmek  ---
Let's finish the review. It's 95% of the way there ;)

While this review has been in progress, python packaging guidelines have
changed (See
https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file). You
should change %build and %install to

%build
%py2_build

%install
py2_install

This should have the exact same effect, but is standard and more concise.

%description is still awkward. If you really want to keep the history part in,
at least remove the paragraph about PDFtk. No need to go into detail about an
alternative project's downsides.

Please add empty lines between each entry in %changelog.

--

I recently gained the sponsorship privileges and I'd be happy to sponsor you
into the packagers group. Please open up a fresh copy of
https://fedoraproject.org/wiki/Packaging:Guidelines, fire up fedora-review, and
do a two-three reviews from
https://fedoraproject.org/PackageReviewStatus/NEW.html, and paste the links
here.

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841
[Bug 177841] Tracker: Review requests from new Fedora packagers who need a
sponsor

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #34 from Ranjan Maitra  ---
(In reply to Zbigniew Jędrzejewski-Szmek from comment #33)
> Let's finish the review. It's 95% of the way there ;)
>
> While this review has been in progress, python packaging guidelines have
> changed (See
> https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file).
> You should change %build and %install to
>
> %build
> %py2_build
>
> %install
> py2_install

Thanks, OK with all, but having trouble with this. How do I add in the
additional arguments?

I used to have the following in the spec file:

%{__python2} setup.py install -O1 --skip-build --root $RPM_BUILD_ROOT

How do I replace this with %{py2_install} -- sorry, I tried everything I could
think of, and the page of guidelines is not completely clear.

Thanks again!

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #35 from Zbigniew Jędrzejewski-Szmek  ---
%py2_install already includes all those arguments (try rpmbuild --eval
%py2_install), so you use it like in the part you quoted above, literally.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #36 from Ranjan Maitra  ---
New files posted:

SRPM: http://maitra.public.iastate.edu/Fedora/pdf-stapler-0.3.3-4.fc23.src.rpm
SPEC: http://maitra.public.iastate.edu/Fedora/pdf-stapler.spec

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Zbigniew Jędrzejewski-Szmek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+

--- Comment #37 from Zbigniew Jędrzejewski-Szmek  ---
Yep, looks good.

Looking at %description again, the last paragraph is now obsolete, pypdf2 has
been packaged.

- license is OK (BSD), license tag matches code
- license file is present, %license is used
- latest version
- package name follows the guidelines
- new python packaging template is used
- builds and install OK
- no scriptlets necessary or present
- provides and requires are OK

rpmlint:
pdf-stapler.noarch: W: spelling-error %description -l en_US opensource -> open
source, open-source, outsource
pdf-stapler.noarch: W: spelling-error %description -l en_US commandline ->
command line, command-line, commandment
pdf-stapler.noarch: W: spelling-error %description -l en_US pypdf -> PDF
pdf-stapler.noarch: W: spelling-error %description -l en_US refactored -> re
factored, re-factored, factored
pdf-stapler.noarch: W: spelling-error %description -l en_US pyPdf -> PDF
pdf-stapler.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/staplelib/stapler.py 644 /usr/bin/env
pdf-stapler.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/staplelib/tests.py 644 /usr/bin/env
pdf-stapler.noarch: W: no-manual-page-for-binary pdf-stapler
pdf-stapler.src: W: spelling-error %description -l en_US opensource -> open
source, open-source, outsource
pdf-stapler.src: W: spelling-error %description -l en_US commandline -> command
line, command-line, commandment
pdf-stapler.src: W: spelling-error %description -l en_US pypdf -> PDF
pdf-stapler.src: W: spelling-error %description -l en_US refactored -> re
factored, re-factored, factored
pdf-stapler.src: W: spelling-error %description -l en_US pyPdf -> PDF
Those can be ignored.

pdf-stapler.src:68: W: macro-in-%changelog %{__python}
pdf-stapler.src:68: W: macro-in-%changelog %{__python_macros}
Please fix those by doubling the macro sign in comments: %%{_python}, etc.
rpm recently warns about this during build and it is very annoying.

The guidelines say that you need to provide python-<modulename> and
python2-<modulename>.
See https://fedoraproject.org/wiki/Packaging:Python#Provides. I'm not sure that
those are
very meaningful in this case, but it doesn't harm to add:
Provides: python2-staplelib = %{version}-%{release}
%{?python_provide:%python_provide python2-%{srcname}}

Only trivial issues remain, please fix those up when uploading the package.
Package is APPROVED.

We'll also need to finish the sponsorship process, before you are able to
upload
the package.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #38 from Ranjan Maitra  ---
New files posted:

SRPM: http://maitra.public.iastate.edu/Fedora/pdf-stapler-0.3.3-5.fc23.src.rpm
SPEC: http://maitra.public.iastate.edu/Fedora/pdf-stapler.spec

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #39 from Ranjan Maitra  ---
Reviewed: https://bugzilla.redhat.com/show_bug.cgi?id=1291008

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #40 from Ranjan Maitra  ---
Reviewed: https://bugzilla.redhat.com/show_bug.cgi?id=1289760

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #41 from Ranjan Maitra  ---
Reviewing: https://bugzilla.redhat.com/show_bug.cgi?id=1074595

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Stuart D Gathman  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #42 from Stuart D Gathman  ---
I added a background overlay feature: 
https://github.com/hellerbarde/stapler/pull/23

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #43 from Zbigniew Jędrzejewski-Szmek  ---
Ranjan, I'm ready to add you to the packagers group, but I'm not sure about the
account details:

17:37 <zbyszek> fasinfo aarem
17:37 <zodbot> User: aarem, Name: Ranjan Maitra, email: [e-mail],
Creation: 2012-01-09, 
               IRC Nick: None, Timezone: UTC, Locale: C, GPG key ID: None,
Status: active
17:37 <zodbot> Approved Groups: cla_fpca cla_done 
17:37 <zbyszek> fasinfo maitra
17:37 <zodbot> User: maitra, Name: Ranjan Maitra, email: [e-mail],
Creation: 
               2015-06-20, IRC Nick: None, Timezone: UTC, Locale: en, GPG key
ID: 038B87FE, 
               Status: active
17:37 <zodbot> Approved Groups: cla_done cla_fpca

Did you create both? Please note that the e-mail address in bugzilla has to
mount the one in FAS. If both are yours, let me know which one you want to use,
and if the second one, change the e-mail to match bugzilla (or vice versa).
Please
terminate the other account
(https://fedoraproject.org/wiki/Account_System#Voluntary).

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #44 from Ranjan Maitra  ---
Thanks! Let us use the first account. I was unaware of its existence:-(.

I have deactivated the second account (though I am not 100% sure about it
because the instructions were not completely clear to me).

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #45 from Zbigniew Jędrzejewski-Szmek  ---
It doesn't seem to have worked, account "maitra" still seems to be active and
to contain full name/email information. Please try again and file a ticket if
it doesn't work.

I've added you to the packagers group. Welcome!
I'm happy to help with any questions or issues you might have in the future.
Please keep reviewing new packages.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Zbigniew Jędrzejewski-Szmek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |POST

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #46 from Jon Ciesla  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/pdf-stapler

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #47 from Fedora Update System  ---
pdf-stapler-0.3.3-5.fc23 has been submitted as an update to Fedora 23.
https://bodhi.fedoraproject.org/updates/FEDORA-2016-aa0c4facc9

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|MODIFIED                    |ON_QA

--- Comment #48 from Fedora Update System  ---
pdf-stapler-0.3.3-5.fc23 has been pushed to the Fedora 23 testing repository.
If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2016-aa0c4facc9

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|POST                        |MODIFIED

--- Comment #47 from Fedora Update System  ---
pdf-stapler-0.3.3-5.fc23 has been submitted as an update to Fedora 23.
https://bodhi.fedoraproject.org/updates/FEDORA-2016-aa0c4facc9

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #49 from Fedora Update System  ---
pdf-stapler-0.3.3-5.fc22 has been pushed to the Fedora 22 testing repository.
If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2016-5ac2c2180a

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #50 from Fedora Update System  ---
pdf-stapler-0.3.3-5.fc23 has been pushed to the Fedora 23 stable repository. If
problems still persist, please make note of it in this bug report.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ON_QA                       |CLOSED
         Resolution|---                         |ERRATA
        Last Closed|                            |2016-01-17 12:51:11

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

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

--- Comment #51 from Fedora Update System  ---
pdf-stapler-0.3.3-5.fc22 has been pushed to the Fedora 22 stable repository. If
problems still persist, please make note of it in this bug report.

[Bug 1234210] Review Request: pdf-stapler - tool for manipulating PDF documents from the command line

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1234210
Bug 1234210 depends on bug 1262470, which changed state.

Bug 1262470 Summary: Review Request: python-PyPDF2 - A pure Python library built as a PDF toolkit
https://bugzilla.redhat.com/show_bug.cgi?id=1262470

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|CLOSED                      |MODIFIED
         Resolution|ERRATA                      |---