https://bugzilla.redhat.com/show_bug.cgi?id=1515701
Bug ID: 1515701 Summary: Review Request: diceware - It will create passphrases which one can remember Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: mail@kushaldas.in QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://kushal.fedorapeople.org/packages/diceware.spec SRPM URL: https://kushal.fedorapeople.org/packages/diceware-0.9.3-1.fc26.src.rpm Description: A simple command line tool which can create simple passphrases which human can remember. Fedora Account System Username: kushal
https://bugzilla.redhat.com/show_bug.cgi?id=1515701
Iwicki Artur fedora@svgames.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@svgames.pl
--- Comment #1 from Iwicki Artur fedora@svgames.pl ---
Summary: It will create passphrases which one can remember
Sounds a bit weird to me. I'd get rid of the "it will" part.
%doc LICENSE
This should be marked as %license.
There's a man page in the source archive. Could you include it in the RPM? Since it doesn't get installed by setup.py, you'll have to do that manually:
install -m 755 -D %{buildroot}%{_mandir}/man1/ install -m 644 -p %{name}.1 %{buildroot}%{_mandir}/man1/
Also, do we package sphinx in Fedora? The project readme says that the man page is autogenerated from a ReStructuredTexT template. As such, we may want to run the generator during the build so we're sure that we don't ship an outdated man page.
https://bugzilla.redhat.com/show_bug.cgi?id=1515701
Robert-André Mauchin zebob.m@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zebob.m@gmail.com
--- Comment #2 from Robert-André Mauchin zebob.m@gmail.com --- - Use PythonHosted for Source0 to avoid dealing with checksum:
Source0: https://files.pythonhosted.org/packages/source/d/%%7Bname%7D/%%7Bname%7D-%%7...
- %{__python3} setup.py build → %py3_build
- %{__python3} setup.py install --skip-build --root %{buildroot} → %py3_install
- Generate the docs:
BuildRequires: python3-sphinx BuildRequires: python3-sphinx_rtd_theme
Then:
%install %py3_install pushd docs PYTHONPATH=%{buildroot}%{python3_sitelib} make html popd
Finally add it to a -doc subpackage:
%package doc Summary: Documentation for Diceware BuildArch: noarch %description doc Diceware is a simple command line tool which can create simple passphrases which human can remember.
This package provides documentation for Diceware.
And:
%files doc %doc docs/_build/html
- Run the tests:
BuildRequires: python3-pytest BuildRequires: python3-pytest-runner
Then:
%check PYTHONPATH=%{buildroot}%{python3_sitelib} py.test-%{python3_version}
- Regenerate the man page:
First add this BR:
BuildRequires: %{_bindir}/rst2man
Then in %install:
rst2man docs/manpage.rst %{buildroot}%{_mandir}/man1/diceware.1
And add it in %files:
%{_mandir}/man1/diceware.1*
- Remove the trailing tabs:
Name: diceware Version: 0.9.3
- Simplify the summary:
Summary: Create passphrases which one can remember
https://bugzilla.redhat.com/show_bug.cgi?id=1515701
--- Comment #3 from kushaldas@gmail.com mail@kushaldas.in --- Updated SRPM: https://kushal.fedorapeople.org/packages/diceware-0.9.3-2.fc26.src.rpm Updated SPEC: https://kushal.fedorapeople.org/packages/diceware.spec
This does not have the docs subpackage as I will first ask the upstream to update the sphinx build Makefile.
https://bugzilla.redhat.com/show_bug.cgi?id=1515701
Parag AN(पराग) panemade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |panemade@gmail.com Assignee|nobody@fedoraproject.org |panemade@gmail.com Flags| |fedora-review+
--- Comment #4 from Parag AN(पराग) panemade@gmail.com --- Looks good to me.
Some suggestions: 1) Change the "%setup -q" to "%autosetup". See how it will benefit -> https://fedoraproject.org/wiki/Packaging:Guidelines#.25autosetup
2) Use %license macro for marking license file %license LICENSE
See https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text
3) Also, good to add docs subpackage as recommended above except you need to use
pushd docs PYTHONPATH=%{buildroot}%{python3_sitelib} sphinx-build-3 -b html -d _build/doctrees . _build/html popd
I have modified your spec and my modifications are ======================================================================= --- diceware.spec 2017-12-27 14:14:49.000000000 +0530 +++ diceware.spec.new 2017-12-27 15:50:19.655108962 +0530 @@ -18,9 +18,20 @@ A simple command line tool which can create simple passphrases which human can remember.
+%package doc +Summary: Documentation for Diceware + +BuildArch: noarch +BuildRequires: python3-sphinx +BuildRequires: python3-sphinx_rtd_theme + +%description doc +A simple command line tool which can create simple passphrases +which human can remember. +
%prep -%setup -q +%autosetup
%build @@ -32,12 +43,23 @@ mkdir -p %{buildroot}%{_mandir}/man1 rst2man docs/manpage.rst %{buildroot}%{_mandir}/man1/diceware.1
+pushd docs +PYTHONPATH=%{buildroot}%{python3_sitelib} sphinx-build-3 -b html -d _build/doctrees . _build/html +popd + +# Remove unneeded build artifacts. +rm -rf docs/_build/.buildinfo +rm -rf docs/_build/.doctrees + %check PYTHONPATH=%{buildroot}%{python3_sitelib} py.test-%{python3_version}
+%files doc +%doc docs/_build/html
%files -%doc LICENSE README.rst +%doc README.rst COPYRIGHT +%license LICENSE %{_bindir}/%{name} %{python3_sitelib}/%{name}* %{_mandir}/man1/diceware.1*
============================================================================
APPROVED (provided you will use above changes) :)
https://bugzilla.redhat.com/show_bug.cgi?id=1515701
--- Comment #5 from kushaldas@gmail.com mail@kushaldas.in --- Updated SRPM: https://kushal.fedorapeople.org/packages/diceware-0.9.3-3.fc26.src.rpm Updated SPEC: https://kushal.fedorapeople.org/packages/diceware.spec
https://bugzilla.redhat.com/show_bug.cgi?id=1515701
--- Comment #6 from Parag AN(पराग) panemade@gmail.com --- Looks good.
https://bugzilla.redhat.com/show_bug.cgi?id=1515701
--- Comment #7 from Gwyn Ciesla limburgher@gmail.com --- (fedrepo-req-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/diceware. You may commit to the branch "f26" in about 10 minutes.
https://bugzilla.redhat.com/show_bug.cgi?id=1515701
--- Comment #8 from Fedora Update System updates@fedoraproject.org --- diceware-0.9.3-3.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-5701a7c419
https://bugzilla.redhat.com/show_bug.cgi?id=1515701
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=1515701
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #9 from Fedora Update System updates@fedoraproject.org --- diceware-0.9.3-3.fc26 has been pushed to the Fedora 26 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-2017-5701a7c419
https://bugzilla.redhat.com/show_bug.cgi?id=1515701
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed| |2018-01-03 16:19:15
--- Comment #10 from Fedora Update System updates@fedoraproject.org --- diceware-0.9.3-3.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report.
package-review@lists.fedoraproject.org