Bug 1379651

Summary: Review Request: python-feedgenerator - Standalone version of Django's feedgenerator module
Product: [Fedora] Fedora Reporter: Ankur Sinha (FranciscoD) <sanjay.ankur>
Component: Package ReviewAssignee: Germano Massullo <germano.massullo>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: urgent    
Version: rawhideCC: bunnyapocalypse, fedora, germano.massullo, jean-baptiste, mrunge, package-review, quantum.analyst, sanjay.ankur
Target Milestone: ---Flags: germano.massullo: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-08-14 20:17:35 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1379149, 1480922    
Attachments:
Description Flags
sample spec file none

Description Ankur Sinha (FranciscoD) 2016-09-27 10:34:43 UTC 
Spec URL: https://ankursinha.fedorapeople.org/feedgenerator/python-feedgenerator.spec

SRPM URL: https://ankursinha.fedorapeople.org/feedgenerator/python-feedgenerator-1.9-1.fc24.src.rpm

Description:
FeedGenerator is a standalone version of Django’s feedgenerator module. It has
evolved over time, including an update for Py3K and numerous other
enhancements.
 

Fedora Account System Username: ankursinha
Comment 1 Matthias Runge 2016-09-27 11:10:45 UTC 
please use
https://files.pythonhosted.org/packages/source/f/feedgenerator/feedgenerator-1.9.tar.gz
instead of your source0 url.

upstream provides tests, please provide a check section
Comment 2 Germano Massullo 2016-09-27 12:25:18 UTC 
Ankur, could you please:

1) replace
%global srcname feedgenerator
with
%global pypi_name feedgenerator
and all srcname occurrencies in the spec file

2) replace
URL: https://pypi.python.org/pypi/%{srcname}
with
URL: https://github.com/getpelican/feedgenerator

3) replace
Source0:    https://pypi.python.org/packages/eb/02/7069b3dbc6ea92f034e07f9f9adc2193cd02d1aedf2cf9ec71150102a964/%{srcname}-%{version}.tar.gz
with
Source0: https://files.pythonhosted.org/packages/source/%(n=%{pypi_name}; echo ${n:0:1})/%{pypi_name}/%{pypi_name}-%{version}.tar.gz

Thank you
Comment 3 Ankur Sinha (FranciscoD) 2016-09-27 13:12:42 UTC 
Done. 

Updated spec/srpm:
https://ankursinha.fedorapeople.org/feedgenerator/python-feedgenerator-1.9-2.fc24.src.rpm
https://ankursinha.fedorapeople.org/feedgenerator/python-feedgenerator.spec


The py2 test runs properly, but the py3 test fails. I'm investigating, but have disabled it for the time being. Would you folks have any hints?

+ /usr/bin/python3 setup.py test
running test
running egg_info
writing top-level names to feedgenerator.egg-info/top_level.txt                                                                                                                                                                      [10/6237]
writing dependency_links to feedgenerator.egg-info/dependency_links.txt
writing feedgenerator.egg-info/PKG-INFO
writing requirements to feedgenerator.egg-info/requires.txt
reading manifest file 'feedgenerator.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'feedgenerator.egg-info/SOURCES.txt'
running build_ext
Traceback (most recent call last):
  File "setup.py", line 58, in <module>
    zip_safe=False,
  File "/usr/lib64/python3.5/distutils/core.py", line 148, in setup
    dist.run_commands()
  File "/usr/lib64/python3.5/distutils/dist.py", line 955, in run_commands
    self.run_command(cmd)
  File "/usr/lib64/python3.5/distutils/dist.py", line 974, in run_command
    cmd_obj.run()
  File "/usr/lib/python3.5/site-packages/setuptools/command/test.py", line 159, in run
    self.with_project_on_sys_path(self.run_tests)
  File "/usr/lib/python3.5/site-packages/setuptools/command/test.py", line 140, in with_project_on_sys_path
    func()
  File "/usr/lib/python3.5/site-packages/setuptools/command/test.py", line 180, in run_tests
    testRunner=self._resolve_as_ep(self.test_runner),
  File "/usr/lib64/python3.5/unittest/main.py", line 93, in __init__
    self.parseArgs(argv)
  File "/usr/lib64/python3.5/unittest/main.py", line 140, in parseArgs
    self.createTests()
  File "/usr/lib64/python3.5/unittest/main.py", line 147, in createTests
    self.module)
  File "/usr/lib64/python3.5/unittest/loader.py", line 219, in loadTestsFromNames
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/usr/lib64/python3.5/unittest/loader.py", line 219, in <listcomp>
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/usr/lib64/python3.5/unittest/loader.py", line 190, in loadTestsFromName
    return self.loadTestsFromModule(obj)
  File "/usr/lib/python3.5/site-packages/setuptools/command/test.py", line 38, in loadTestsFromModule
    tests.append(self.loadTestsFromName(submodule))
  File "/usr/lib64/python3.5/unittest/loader.py", line 153, in loadTestsFromName
    module = __import__(module_name)
  File "/builddir/build/BUILD/feedgenerator-1.9/tests_feedgenerator/usage_example.py", line 36, in <module>
    feed.write(fh, 'utf-8')
  File "/builddir/build/BUILD/feedgenerator-1.9/feedgenerator/django/utils/feedgenerator.py", line 223, in write
    self.add_root_elements(handler)
  File "/builddir/build/BUILD/feedgenerator-1.9/feedgenerator/django/utils/feedgenerator.py", line 241, in add_root_elements
    handler.addQuickElement("description", self.feed['description'])
  File "/builddir/build/BUILD/feedgenerator-1.9/feedgenerator/django/utils/xmlutils.py", line 15, in addQuickElement
    self.characters(contents)
  File "/usr/lib64/python3.5/xml/sax/saxutils.py", line 214, in characters
    self._write(escape(content))


I tried it in a clean virtual env and it seemed to work properly, so I may have messed something up in the spec.

Cheers,
Ankur
Comment 4 Matthias Runge 2016-09-27 13:32:25 UTC 
feedgenerator is a fork of django. You 

Django's unit tests are fine in koji: http://pkgs.fedoraproject.org/cgit/rpms/python-django.git/tree/python-django.spec#n229
Comment 5 Germano Massullo 2016-09-27 14:07:17 UTC 
Created attachment 1205204 [details]
sample spec file

License file is missing.
1) Go to upstream Github website, fill a bugreport ticket saying that in Pypi package the LICENSE file is missing.
2) Download
https://raw.githubusercontent.com/getpelican/feedgenerator/master/LICENSE
and include it into SOURCES folder when you make the src rpm file.
3) Add to the spec file
Source1: https://raw.githubusercontent.com/getpelican/feedgenerator/master/LICENSE
then fill the missing macros that allows you to correctly include the license file into.
Take as example attached python-django-jsonfield.spec file
line numbers 62, 87, 93
Comment 6 Germano Massullo 2016-09-27 14:32:51 UTC 
And sorry if I seemed to be rude but I was doing 3 different things at the same time :D
Comment 7 Ankur Sinha (FranciscoD) 2016-09-27 15:13:56 UTC 
(In reply to Germano Massullo from comment #5)
> Created attachment 1205204 [details]
> sample spec file
> 
> License file is missing.
> 1) Go to upstream Github website, fill a bugreport ticket saying that in
> Pypi package the LICENSE file is missing.


I did that before I submitted the package review request already ;)

https://github.com/getpelican/feedgenerator/issues/10


> 2) Download
> https://raw.githubusercontent.com/getpelican/feedgenerator/master/LICENSE
> and include it into SOURCES folder when you make the src rpm file.

Will do and update the spec. Any clues over the failed test? :/
Comment 8 Ankur Sinha (FranciscoD) 2016-09-27 15:15:47 UTC 
(In reply to Matthias Runge from comment #4)
> feedgenerator is a fork of django. You 
> 
> Django's unit tests are fine in koji:
> http://pkgs.fedoraproject.org/cgit/rpms/python-django.git/tree/python-django.
> spec#n229

I'll check this up, Matthias. Must be something tiny. (hopefully, rather)
Comment 9 Matthias Runge 2016-10-27 13:11:41 UTC 
maybe we should backport the fix they did in django-1.10 to django-1.9 rather than creating a new package, that is basically a (half) duplicate of django? I still don't see much value in this package (compared to django itself)
Comment 10 Ankur Sinha (FranciscoD) 2016-10-27 14:12:29 UTC 
(In reply to Matthias Runge from comment #9)
> maybe we should backport the fix they did in django-1.10 to django-1.9
> rather than creating a new package, that is basically a (half) duplicate of
> django? I still don't see much value in this package (compared to django
> itself)

Sure, if you're willing to carry the patch? :D
Comment 11 Matthias Runge 2016-10-31 13:08:04 UTC 
(In reply to Ankur Sinha (FranciscoD) from comment #10)

> 
> Sure, if you're willing to carry the patch? :D

Provide it ;-) The diff between django and feedparser is quite minimal.
Comment 12 Ankur Sinha (FranciscoD) 2016-11-01 11:54:29 UTC 
Ha! I'll provide it by the weekend hopefully. Should just be the one diff that carries the feedparser fix between the two django versions, or should I check the differences between feedparser and django too?
Comment 13 Matthias Runge 2016-11-01 15:48:11 UTC 
Ankur, since the issue was only about feedparser, I would accept a backport of the fix in feedparser from a later django version.
Comment 14 Adam Williamson 2017-02-07 13:39:29 UTC 
The pelican package explicitly requires a module called 'feedgenerator' now, so either that needs patching or this needs packaging.
Comment 15 Georg Sauthoff 2017-08-13 10:47:20 UTC 
(In reply to Matthias Runge from comment #9)
> maybe we should backport the fix they did in django-1.10 to django-1.9
> rather than creating a new package, that is basically a (half) duplicate of
> django? I still don't see much value in this package (compared to django
> itself)

concrete value: if the pelican package would use the pelican-feedgenerator package as intended and tested by the pelican developers those 2 bugs would have been prevented from happening:

- Random attribute order in generated feeds (Bug 421185)
- python-pelican is shipped with an old version of feedgenerator.py which leads to unexpected behaviour (Bug 1480922)
Comment 16 Demetris M 2017-08-16 11:58:03 UTC 
I did my best to review the spec in Comment #3, but bare with me because that's the first time I'm doing this.

The package builds successfully, and if python-pelican is rebuilt with the django substitution removed, python-pelican works as expected. 

The only problem I caught is the lack of LICENSE file, which is something already reported. 

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

Generic:
[!]: License file installed when any subpackage combination is installed.
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license. [It doesn't contain LICENSE]
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated". 23 files have unknown license. Detailed
     output of licensecheck in /home/blob/Desktop/pelican-feedparser
     /review-python-feedgenerator/licensecheck.txt
[?]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[?]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[?]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[?]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[?]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 20480 bytes in 2 files.
[?]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[?]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[x]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[?]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     python2-feedgenerator , python3-feedgenerator
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[-]: Package should compile and build into binary rpms on all supported
     architectures. [Is noarch]
[x]: %check is present and all tests pass. [python2 tests]
[?]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: No rpmlint messages.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: python2-feedgenerator-1.9-2.fc26.noarch.rpm
          python3-feedgenerator-1.9-2.fc26.noarch.rpm
          python-feedgenerator-1.9-2.fc26.src.rpm
python-feedgenerator.src:57: W: macro-in-comment %{__python3}
3 packages and 0 specfiles checked; 0 errors, 1 warnings.




Rpmlint (installed packages)
----------------------------
sh: /usr/bin/python: No such file or directory
2 packages and 0 specfiles checked; 0 errors, 0 warnings.



Requires
--------
python3-feedgenerator (rpmlib, GLIBC filtered):
    python(abi)

python2-feedgenerator (rpmlib, GLIBC filtered):
    python(abi)



Provides
--------
python3-feedgenerator:
    python3-feedgenerator
    python3.6dist(feedgenerator)
    python3dist(feedgenerator)

python2-feedgenerator:
    python-feedgenerator
    python2-feedgenerator
    python2.7dist(feedgenerator)
    python2dist(feedgenerator)



Source checksums
----------------
https://files.pythonhosted.org/packages/source/f/feedgenerator/feedgenerator-1.9.tar.gz :
  CHECKSUM(SHA256) this package     : 5ae05daa9cfa47fa406ee4744d0b7fa1c8a05a7a47ee0ad328ddf55327cfb106
  CHECKSUM(SHA256) upstream package : 5ae05daa9cfa47fa406ee4744d0b7fa1c8a05a7a47ee0ad328ddf55327cfb106


Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -n python-feedgenerator
Buildroot used: fedora-26-x86_64
Active plugins: Python, Generic, Shell-api
Disabled plugins: Java, C/C++, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
Comment 17 jibecfed 2017-09-17 11:45:24 UTC 
Licence issue is fixed: https://github.com/getpelican/feedgenerator/issues/10

This thread explains the reason why the Pelican maintainer created a new project for a standalone feedgenerator: https://github.com/getpelican/pelican/issues/2202

Here is copy/paste for history:
> Pelican already uses feedgenerator in a way that produces correct output.

> Pelican needed, and still needs, a standalone Pip-installable package for feedgenerator. No such package existed, so we created one years ago. When doing so, we needed a source code repository from which to build said package — a repo that isn't burdened by all the rest of Django, none of which applies to Pelican. That's why feedgenerator exists.
> When issues came up that needed fixing for Pelican's purposes, we fixed them in feedgenerator. As is the nature of open source, anyone is welcome to submit pull requests containing those fixes to Django's repository.
> Hopefully that clears up some of the history. I'm open to suggestions and would like to help if I can, but so far I have not yet heard anything sensible/actionable from the perspective of Pelican's project management.

Please help us to have blogs that includes the whole content, I really dislike forcing my readers to go on my website to access information.

Have a nice day, and thanks for your actions!
Comment 18 Demetris M 2017-11-05 20:54:26 UTC 
Upstream issues are resolved, including LICENSE, and the spec file works correctly. Is anything else stopping inclusion at the moment?
Comment 19 Elliott Sales de Andrade 2018-01-01 20:55:31 UTC 
Can you provide an update Ankur?
Comment 20 Ankur Sinha (FranciscoD) 2018-01-02 11:47:19 UTC 
Hiya!

Sorry, missed this one. I'll have the updated spec/srpm up in a few!

Cheers!
Comment 21 Ankur Sinha (FranciscoD) 2018-01-02 13:06:58 UTC 
Here are the updated spec/srpm:

https://ankursinha.fedorapeople.org/feedgenerator/python-feedgenerator.spec

https://ankursinha.fedorapeople.org/feedgenerator/python-feedgenerator-1.9-2.fc24.src.rpm

I've fetched the license file from the repo and added it to the package manualy here. When a new release is made, this will no longer be required. I've also fixed the tests which now run successfully.

Test scratch build here:

https://koji.fedoraproject.org/koji/taskinfo?taskID=23970114

Germano, please do take a look when you have a few cycles to spare :)

Cheers!
Ankur
Comment 22 Germano Massullo 2018-07-24 08:54:22 UTC 
(In reply to Ankur Sinha (FranciscoD) from comment #21)
> Here are the updated spec/srpm:
> 
> https://ankursinha.fedorapeople.org/feedgenerator/python-feedgenerator.spec
> 
> https://ankursinha.fedorapeople.org/feedgenerator/python-feedgenerator-1.9-2.
> fc24.src.rpm

spec file in the SRPM is not the same of https://ankursinha.fedorapeople.org/feedgenerator/python-feedgenerator.spec

Could you please regenerate the SRPM file?
Comment 23 Ankur Sinha (FranciscoD) 2018-07-24 16:51:21 UTC 
Updated spec/srpm:

https://ankursinha.fedorapeople.org/python-feedgenerator/python-feedgenerator.spec

https://ankursinha.fedorapeople.org/python-feedgenerator/python-feedgenerator-1.9-3.fc28.src.rpm
Comment 24 Chris King 2018-07-24 20:07:45 UTC 
Just FYI I am not able to actually review your package as I am not yet sponsored so this is an unofficial review.

LGTM but I do not see the license files anywhere, despite you saying that was fixed and the change that seems to have happened upstream. The MANIFEST.in file does not reflect the changes here https://github.com/getpelican/feedgenerator/commit/6582cd4bb3949aad2d1aa7d96c2c90c1ba9cf954, maybe the source0 is outdated or I made a mistake?

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[!]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[!]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated". 23 files have unknown license. Detailed
     output of licensecheck in /home/christopher/1379651-python-
     feedgenerator/licensecheck.txt
[!]: License file installed when any subpackage combination is installed.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[-]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 20480 bytes in 2 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: No rpmlint messages.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[x]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     python2-feedgenerator , python3-feedgenerator
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[-]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: python2-feedgenerator-1.9-3.fc29.noarch.rpm
          python3-feedgenerator-1.9-3.fc29.noarch.rpm
          python-feedgenerator-1.9-3.fc29.src.rpm
3 packages and 0 specfiles checked; 0 errors, 0 warnings.




Rpmlint (installed packages)
----------------------------
sh: /usr/bin/python: No such file or directory
python3-feedgenerator.noarch: W: invalid-url URL: https://github.com/getpelican/feedgenerator <urlopen error [Errno -2] Name or service not known>
python2-feedgenerator.noarch: W: invalid-url URL: https://github.com/getpelican/feedgenerator <urlopen error [Errno -2] Name or service not known>
2 packages and 0 specfiles checked; 0 errors, 2 warnings.



Requires
--------
python3-feedgenerator (rpmlib, GLIBC filtered):
    python(abi)

python2-feedgenerator (rpmlib, GLIBC filtered):
    python(abi)



Provides
--------
python3-feedgenerator:
    python3-feedgenerator
    python3.7dist(feedgenerator)
    python3dist(feedgenerator)

python2-feedgenerator:
    python-feedgenerator
    python2-feedgenerator
    python2.7dist(feedgenerator)
    python2dist(feedgenerator)



Source checksums
----------------
https://files.pythonhosted.org/packages/source/f/feedgenerator/feedgenerator-1.9.tar.gz :
  CHECKSUM(SHA256) this package     : 5ae05daa9cfa47fa406ee4744d0b7fa1c8a05a7a47ee0ad328ddf55327cfb106
  CHECKSUM(SHA256) upstream package : 5ae05daa9cfa47fa406ee4744d0b7fa1c8a05a7a47ee0ad328ddf55327cfb106


Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -b 1379651
Buildroot used: fedora-rawhide-x86_64
Active plugins: Python, Generic, Shell-api
Disabled plugins: Java, C/C++, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
Comment 25 Chris King 2018-07-24 20:29:41 UTC 
> python3-feedgenerator.noarch: W: invalid-url URL:
> https://github.com/getpelican/feedgenerator <urlopen error [Errno -2] Name
> or service not known>
> python2-feedgenerator.noarch: W: invalid-url URL:
> https://github.com/getpelican/feedgenerator <urlopen error [Errno -2] Name
> or service not known>

I don't know why my rpmlint is complaining about this link, it is a perfectly valid URL.
Comment 26 Ankur Sinha (FranciscoD) 2018-07-24 21:55:15 UTC 
Hi Chris!

Thank you for the review! Unfortunately upstream have not made a new release since updating the Manifest, so the pypi tar does not include the License file yet. I've used the tarball from github now, and that includes all the required files.

Update spec/srpm:

https://ankursinha.fedorapeople.org/python-feedgenerator/python-feedgenerator.spec

https://ankursinha.fedorapeople.org/python-feedgenerator/python-feedgenerator-1.9-4.fc28.src.rpm

Cheers!
Ankur
Comment 27 Chris King 2018-07-25 14:37:18 UTC 
With the added license it LGTM. If I were able to, I'd approve this package.

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "BSD 3-clause "New" or "Revised" License", "Unknown or
     generated". 16 files have unknown license. Detailed output of
     licensecheck in /home/christopher/1379651-python-
     feedgenerator/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 20480 bytes in 2 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: No rpmlint messages.
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[x]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     python2-feedgenerator , python3-feedgenerator
[?]: Package functions as described.
[x]: Latest version is packaged.
[!]: Package does not include license text files separate from upstream.
[x]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: python2-feedgenerator-1.9-4.fc29.noarch.rpm
          python3-feedgenerator-1.9-4.fc29.noarch.rpm
          python-feedgenerator-1.9-4.fc29.src.rpm
3 packages and 0 specfiles checked; 0 errors, 0 warnings.




Rpmlint (installed packages)
----------------------------
sh: /usr/bin/python: No such file or directory
python3-feedgenerator.noarch: W: invalid-url URL: https://github.com/getpelican/feedgenerator <urlopen error [Errno -2] Name or service not known>
python2-feedgenerator.noarch: W: invalid-url URL: https://github.com/getpelican/feedgenerator <urlopen error [Errno -2] Name or service not known>
2 packages and 0 specfiles checked; 0 errors, 2 warnings.



Requires
--------
python3-feedgenerator (rpmlib, GLIBC filtered):
    python(abi)

python2-feedgenerator (rpmlib, GLIBC filtered):
    python(abi)



Provides
--------
python3-feedgenerator:
    python3-feedgenerator
    python3.7dist(feedgenerator)
    python3dist(feedgenerator)

python2-feedgenerator:
    python-feedgenerator
    python2-feedgenerator
    python2.7dist(feedgenerator)
    python2dist(feedgenerator)



Source checksums
----------------
https://github.com/getpelican/feedgenerator/archive/6b6572cfd3e7f40993204af5aa70aab443a5b63c/feedgenerator-6b6572c.tar.gz :
  CHECKSUM(SHA256) this package     : 8381f02c05b0bad4bb2d26188f4dc6e4082874c69e2b2237d4e19e34675e9ec8
  CHECKSUM(SHA256) upstream package : 8381f02c05b0bad4bb2d26188f4dc6e4082874c69e2b2237d4e19e34675e9ec8


Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -b 1379651
Buildroot used: fedora-rawhide-x86_64
Active plugins: Python, Generic, Shell-api
Disabled plugins: Java, C/C++, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
Comment 28 Chris King 2018-07-25 14:57:33 UTC 
I'm not sure if assigning this to me is a sign that I've been sponsored or simply an accident...

Just to clarify again, I am not yet (to my knowledge) sponsored, so I don't believe that I should have been assigned here.
Comment 29 Ankur Sinha (FranciscoD) 2018-07-25 16:19:17 UTC 
Hi Chris,

That's OK. Just an oversight. You can look at your FAS account to see the status of your sponsorship. You can also use zodbot in #fedora-devel to get information on your account: https://fedoraproject.org/wiki/Zodbot

Germano: I've reset the assignee field (since it cannot be assigned to Chris) and cleared the review flag too. If you have the time to continue the review, please re-assign them, otherwise someone else can take it up. I expect it should be a rather short review now that Chris has checked the package up already.


Thanks, everyone!
Ankur
Comment 30 Germano Massullo 2018-07-29 18:17:23 UTC 
The package ships a bundled python-six library, version 1.1.0 (from year 2011!)
Could you please try unbundling it and check if feedgenerator works using the Fedora python-six package?
Comment 31 Ankur Sinha (FranciscoD) 2018-08-01 07:39:08 UTC 
Hi Germano,


I missed that! I've unbundled it and it builds just fine:

https://ankursinha.fedorapeople.org/python-feedgenerator/python-feedgenerator.spec

https://ankursinha.fedorapeople.org/python-feedgenerator/python-feedgenerator-1.9-5.fc28.src.rpm

Thanks!
Ankur
Comment 32 Germano Massullo 2018-08-01 10:15:28 UTC 
(In reply to Ankur Sinha (FranciscoD) from comment #31)
> I missed that! I've unbundled it and it builds just fine:

Ok, now you must add python-six (use the proper Python 2 and 3 macros) as runtime requirement with tag Requires:
Comment 33 Germano Massullo 2018-08-01 10:17:54 UTC 
(In reply to Germano Massullo from comment #32)
> (In reply to Ankur Sinha (FranciscoD) from comment #31)
> > I missed that! I've unbundled it and it builds just fine:
> 
> Ok, now you must add python-six (use the proper Python 2 and 3 macros) as
> runtime requirement with tag Requires:

This is a bit obsolete example, but it's just to let you know where "Requires:" should go

https://src.fedoraproject.org/rpms/python-netdiff/blob/master/f/python-netdiff.spec
Comment 34 Ankur Sinha (FranciscoD) 2018-08-01 10:50:42 UTC 
Added:

https://ankursinha.fedorapeople.org/python-feedgenerator/python-feedgenerator.spec

https://ankursinha.fedorapeople.org/python-feedgenerator/python-feedgenerator-1.9-6.fc28.src.rpm
Comment 35 Germano Massullo 2018-08-01 12:09:47 UTC 
Package approved




Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "BSD (3 clause)", "Unknown or generated". 15 files have unknown
     license.
[x]: License file installed when any subpackage combination is installed.
[x]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/lib/python3.7/site-
     packages, /usr/lib/python3.7
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 20480 bytes in 2 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: No rpmlint messages.
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[-]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     python2-feedgenerator , python3-feedgenerator
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).


Rpmlint
-------
Checking: python2-feedgenerator-1.9-6.fc29.noarch.rpm
          python3-feedgenerator-1.9-6.fc29.noarch.rpm
          python-feedgenerator-1.9-6.fc29.src.rpm
3 packages and 0 specfiles checked; 0 errors, 0 warnings.




Rpmlint (installed packages)
----------------------------
sh: /usr/bin/python: No such file or directory
python3-feedgenerator.noarch: W: invalid-url URL: https://github.com/getpelican/feedgenerator <urlopen error [Errno -2] Name or service not known>
python2-feedgenerator.noarch: W: invalid-url URL: https://github.com/getpelican/feedgenerator <urlopen error [Errno -2] Name or service not known>
2 packages and 0 specfiles checked; 0 errors, 2 warnings.



Requires
--------
python3-feedgenerator (rpmlib, GLIBC filtered):
    python(abi)
    python3dist(pytz)
    python3dist(six)

python2-feedgenerator (rpmlib, GLIBC filtered):
    python(abi)
    python2dist(pytz)
    python2dist(six)



Provides
--------
python3-feedgenerator:
    python3-feedgenerator
    python3.7dist(feedgenerator)
    python3dist(feedgenerator)

python2-feedgenerator:
    python-feedgenerator
    python2-feedgenerator
    python2.7dist(feedgenerator)
    python2dist(feedgenerator)



Source checksums
----------------
https://github.com/getpelican/feedgenerator/archive/6b6572cfd3e7f40993204af5aa70aab443a5b63c/feedgenerator-6b6572c.tar.gz :
  CHECKSUM(SHA256) this package     : 8381f02c05b0bad4bb2d26188f4dc6e4082874c69e2b2237d4e19e34675e9ec8
  CHECKSUM(SHA256) upstream package : 8381f02c05b0bad4bb2d26188f4dc6e4082874c69e2b2237d4e19e34675e9ec8


Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -rn /home/user/rpmbuild/SRPMS/python-feedgenerator-1.9-6.fc28.src.rpm -m fedora-rawhide-x86_64
Buildroot used: fedora-rawhide-x86_64
Active plugins: Python, Generic, Shell-api
Disabled plugins: Java, C/C++, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
Comment 36 Gwyn Ciesla 2018-08-01 14:02:44 UTC 
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/python-feedgenerator
Comment 37 Fedora Update System 2018-08-02 07:44:33 UTC 
python-feedgenerator-1.9-6.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-99c6bab44c
Comment 38 Fedora Update System 2018-08-02 07:44:45 UTC 
python-feedgenerator-1.9-6.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-0d2c427d03
Comment 39 Fedora Update System 2018-08-02 14:30:09 UTC 
python-feedgenerator-1.9-6.fc27 has been pushed to the Fedora 27 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-2018-99c6bab44c
Comment 40 Fedora Update System 2018-08-02 17:17:19 UTC 
python-feedgenerator-1.9-6.fc28 has been pushed to the Fedora 28 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-2018-0d2c427d03
Comment 41 Fedora Update System 2018-08-14 20:17:35 UTC 
python-feedgenerator-1.9-6.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report.
Comment 42 Fedora Update System 2018-08-14 21:08:24 UTC 
python-feedgenerator-1.9-6.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.