[Bug 1219288] New: Review Request: python-marshmallow - Python library for converting complex datatypes to and from primitive types
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
Bug ID: 1219288 Summary: Review Request: python-marshmallow - Python library for converting complex datatypes to and from primitive types Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: dcallagh@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://fedorapeople.org/~dcallagh/python-marshmallow/python-marshmallow.spe... SRPM URL: https://fedorapeople.org/~dcallagh/python-marshmallow/python-marshmallow-1.2... Description: Marshmallow is a framework-agnostic library for converting complex datatypes, such as objects, to and from primitive Python datatypes.
Marshmallow schemas can be used to: * Validate input data. * Deserialize input data to app-level objects. * Serialize app-level objects to primitive Python types. The serialized objects can then be rendered to standard formats such as JSON for use in an HTTP API. Fedora Account System Username: dcallagh
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
Julien Enselme jujens@jujens.eu changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |jujens@jujens.eu Assignee|nobody@fedoraproject.org |jujens@jujens.eu Flags| |fedora-review?
--- Comment #1 from Julien Enselme jujens@jujens.eu --- Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
Issues: ======= - 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. Note: License file LICENSE is marked as %doc instead of %license See: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
===== 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. [!]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "MIT/X11 (BSD like)", "Unknown or generated". 58 files have unknown license. Detailed output of licensecheck in /tmp/1219288 -python-marshmallow/licensecheck.txt [X]: License file installed when any subpackage combination is installed. [!]: 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. []: 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 61440 bytes in 8 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: 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. [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 python3-marshmallow [X]: Package functions as described. [!]: 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. [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]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [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]: 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: python-marshmallow-1.2.6-1.fc23.noarch.rpm python3-marshmallow-1.2.6-1.fc23.noarch.rpm python-marshmallow-1.2.6-1.fc23.src.rpm python-marshmallow.noarch: W: spelling-error Summary(en_US) datatypes -> datatype, data types, data-types python-marshmallow.noarch: W: spelling-error %description -l en_US datatypes -> datatype, data types, data-types python-marshmallow.noarch: W: spelling-error %description -l en_US schemas -> schema, sachems, schemes python3-marshmallow.noarch: W: spelling-error Summary(en_US) datatypes -> datatype, data types, data-types python3-marshmallow.noarch: W: spelling-error %description -l en_US datatypes -> datatype, data types, data-types python3-marshmallow.noarch: W: spelling-error %description -l en_US schemas -> schema, sachems, schemes python-marshmallow.src: W: spelling-error Summary(en_US) datatypes -> datatype, data types, data-types python-marshmallow.src: W: spelling-error %description -l en_US datatypes -> datatype, data types, data-types python-marshmallow.src: W: spelling-error %description -l en_US schemas -> schema, sachems, schemes 3 packages and 0 specfiles checked; 0 errors, 9 warnings.
Rpmlint (installed packages) ---------------------------- python-marshmallow.noarch: W: spelling-error Summary(en_US) datatypes -> datatype, data types, data-types python-marshmallow.noarch: W: spelling-error %description -l en_US datatypes -> datatype, data types, data-types python-marshmallow.noarch: W: spelling-error %description -l en_US schemas -> schema, sachems, schemes python3-marshmallow.noarch: W: spelling-error Summary(en_US) datatypes -> datatype, data types, data-types python3-marshmallow.noarch: W: spelling-error %description -l en_US datatypes -> datatype, data types, data-types python3-marshmallow.noarch: W: spelling-error %description -l en_US schemas -> schema, sachems, schemes 2 packages and 0 specfiles checked; 0 errors, 6 warnings.
Requires -------- python-marshmallow (rpmlib, GLIBC filtered): python(abi) python-dateutil python3-dateutil
python3-marshmallow (rpmlib, GLIBC filtered): python(abi)
Provides -------- python-marshmallow: python-marshmallow
python3-marshmallow: python3-marshmallow
Source checksums ---------------- https://github.com/marshmallow-code/marshmallow/archive/c786c58242028c6742de... : CHECKSUM(SHA256) this package : fe4306e085823a63cf5d7fc37d53ff0364aab9eaed48855dc108e0415bef1a12 CHECKSUM(SHA256) upstream package : fe4306e085823a63cf5d7fc37d53ff0364aab9eaed48855dc108e0415bef1a12
Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20 Command line :/usr/bin/fedora-review -b 1219288 -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, Ruby Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
- Use %license for license: https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/Li... - License is MIT not BSD - Two libs are bundled: ordereddict and orderedset. ordereddict is there only for compatibility with python 2.6. I think you can safely remove it and path the files to use OrderedDict from collections. You can also package this library. orderedset is not present at this time in the repository of fedora. - Package lastest version (2.0.0b3)
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #2 from Julien Enselme jujens@jujens.eu --- Any progress on addressing the problems I pointed out?
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
Miroslav Suchý msuchy@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |dcallagh@redhat.com, | |msuchy@redhat.com Flags| |needinfo?(dcallagh@redhat.c | |om)
--- Comment #3 from Miroslav Suchý msuchy@redhat.com --- Dan, are you willing to continue with this review? I am interrested in that package too and I am willing to continue instead of you if you do not have time.
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
Dan Callaghan dcallagh@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(dcallagh@redhat.c | |om) |
--- Comment #4 from Dan Callaghan dcallagh@redhat.com --- Sorry for my silence on this bz. Currently I am not using marshmallow for anything, and I have been short on time to finish this off. So if you would like to take over Miroslav, please feel free.
You can put me down as a co-maintainer if you'd like, I am happy to help out.
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
Miroslav Suchý msuchy@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |1260575
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1260575 [Bug 1260575] Review Request: python-ordered-set - A Custom MutableSet that remembers its order
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #5 from Miroslav Suchý msuchy@redhat.com --- SRPM: http://miroslav.suchy.cz/fedora/python-marshmallow-2.0.0b5-2.fc22.src.rpm SPEC: http://miroslav.suchy.cz/fedora/python-marshmallow.spec
I unbundled those two modules (the second one is under review in bug 1260575).
There is two false-positives from fedora-review 1) about license in doc section, which is just nearly empty file 2) about macros in comments in %files section - this is something new to me and discussed at http://lists.rpm.org/pipermail/rpm-list/2015-September/001766.html right now.
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #6 from Miroslav Suchý msuchy@redhat.com --- SRPM: http://miroslav.suchy.cz/fedora/python-marshmallow-2.0.0b5-2.fc22.src.rpm SPEC: http://miroslav.suchy.cz/fedora/python-marshmallow.spec
I uncommented that one line in %files which cause rpmbuild to emit warning about files listed twice, but that is rpm bug 728959
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #7 from Miroslav Suchý msuchy@redhat.com --- SRPM: http://miroslav.suchy.cz/fedora/python-marshmallow-2.0.0b5-3.fc22.src.rpm SPEC: http://miroslav.suchy.cz/fedora/python-marshmallow.spec
Correct links.
https://bugzilla.redhat.com/show_bug.cgi?id=1219288 Bug 1219288 depends on bug 1260575, which changed state.
Bug 1260575 Summary: Review Request: python-ordered-set - A Custom MutableSet that remembers its order https://bugzilla.redhat.com/show_bug.cgi?id=1260575
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |NEXTRELEASE
https://bugzilla.redhat.com/show_bug.cgi?id=1219288 Bug 1219288 depends on bug 1260575, which changed state.
Bug 1260575 Summary: Review Request: python-ordered-set - A Custom MutableSet that remembers its order https://bugzilla.redhat.com/show_bug.cgi?id=1260575
What |Removed |Added ---------------------------------------------------------------------------- Status|CLOSED |ON_QA Resolution|NEXTRELEASE |---
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #8 from Julien Enselme jujens@jujens.eu --- - In the python3 section of BR, you wrote 'BuildRequires: python-ordered-set' but I think you meant 'BuildRequires: python3-ordered-set' (I cannot launch tests for python3 because this dependency is missing). - You can simplify:
%if %{with python3} %package -n python3-%{upstream_name} Summary: Python 3 library for converting complex datatypes to and from primitive types %if %{with python3} Requires: python3-dateutil Requires: python3-ordered-set %endif
into (you already are in a python3 only section, no need to specify it again)
%if %{with python3} %package -n python3-%{upstream_name} Summary: Python 3 library for converting complex datatypes to and from primitive types Requires: python3-dateutil Requires: python3-ordered-set
- The use of %py3dir is deprecated. Python2 and python3 packages should be built from the same folder. The build and installation process should respectively rely on %py2_build/%py3build and %py2_install/%py3_install. See https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file for an example.
- I think it would be clearer to remove ordereddict and orderedset in %prep rather than in %install after the package is installed
- Maybe move part of the doc (%{_docdir}/python-%{upstream_name}) into a -doc subpackage which could be used for both python2 and python3.
- You are packaging a pre-release (the latest stable release would be fine too), so you release tag is incorrect. See: https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #9 from Miroslav Suchý msuchy@redhat.com ---
BuildRequires: python-ordered-set
Fixed. Nice catch. Thanks.
You can simplify
Simplified.
The use of %py3dir is deprecated.
Fixed.
it would be clearer to remove ordereddict and orderedset in %prep
Moved.
Maybe move part of the doc...
-doc subpackages created.
- You are packaging a pre-release (the latest stable release would be fine too), so you release tag is incorrect.
??? Not sure I follow here. The packaged version is the latest stable, i.e. 2.0.0b5. Only difference is that I do not use tar.gz from PyPi (because it does not include tests and I use github tar ball. But I use the commit from which the release was made. So it is not pre-release tar file.
Updated: SRPM: http://miroslav.suchy.cz/fedora/python-marshmallow-2.0.0b5-4.fc22.src.rpm SPEC: http://miroslav.suchy.cz/fedora/python-marshmallow.spec
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #10 from Julien Enselme jujens@jujens.eu --- - I see you created two doc packages, one for python2 and one for python3. I don't know if it is necessary, maybe python-marshmallow-doc is enough. I let you judge that.
The packaged version is the latest stable, i.e. 2.0.0b5
From what I understand from the README ("Get it now" section, `pip install -U
marshmallow --pre`) and what I can g, the "b" means beta. Hence it is a pre-release package that should follow the release version number as describe in https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages
However, since you are using git and have a git commit, you should also use that. So here, the release number should be: 0.4.gita8b3385
Once the software is out of beta (so version 2.0.0), release number should be: 1.gita8b3385
Furthermore, starting from now, the version should be 2.0.0.
Is it clearer this time?
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #11 from Miroslav Suchý msuchy@redhat.com --- Both https://pypi.python.org/pypi/marshmallow http://marshmallow.readthedocs.org/en/latest/ state 2.0.0b5 as release. I do not know what "b" stands for. For me - it is released, then it is release number. I won't judge the upstream for the numbering.
However if this is blocker for you I can put the short git hash in Release number.
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #12 from Julien Enselme jujens@jujens.eu ---
state 2.0.0b5 as release.
I don't think they were design to state anything else. If I look at the milestone on github: https://github.com/marshmallow-code/marshmallow/milestones I think this confirms that the b stands for beta.
However if this is blocker for you I can put the short git hash in Release number.
From what I understand from the guidelines, no matter what "b" means, we must
treat it as a pre-release. I am not doing it for the sake of arguing, I just try to do the thing right. I may, however, misunderstand the guidelines, in that case, I will be happy to approve this package if you prove me wrong.
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #13 from Miroslav Suchý msuchy@redhat.com --- (In reply to Julien Enselme from comment #12)
I don't think they were design to state anything else. If I look at the milestone on github: https://github.com/marshmallow-code/marshmallow/milestones I think this confirms that the b stands for beta.
This is fair argument.
Updated: SRPM: http://miroslav.suchy.cz/fedora/python-marshmallow-2.0.0-0.5.a8b3385.fc22.sr... SPEC: http://miroslav.suchy.cz/fedora/python-marshmallow.spec
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #14 from Julien Enselme jujens@jujens.eu --- Almost there: you cannot just put the short commit, you must specify the software (here git): 2.0.0-0.5.gita8b3385
Caught by fedora-review: [ ]: Package must own all directories that it creates. Note: Directories without known owners: /usr/share/doc/python3-marshmallow, /usr/share/doc/python-marshmallow
I think it comes from the fact that %{_docdir} is not defined. It could be %{_datadir}/doc though.
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #15 from Miroslav Suchý msuchy@redhat.com --- Updated: SRPM: http://miroslav.suchy.cz/fedora/python-marshmallow-2.0.0-0.6.a8b3385.fc22.sr... SPEC: http://miroslav.suchy.cz/fedora/python-marshmallow.spec
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
Julien Enselme jujens@jujens.eu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #16 from Julien Enselme jujens@jujens.eu --- Looks good. Beware, in the changelog, you still have 2.0.0-0.5.a8b3385 instead of 2.0.0-0.5.gita8b3385
Approved.
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #17 from Miroslav Suchý msuchy@redhat.com --- Thank you for the review.
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
Miroslav Suchý msuchy@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #18 from Miroslav Suchý msuchy@redhat.com --- New Package SCM Request ======================= Package Name: python-marshmallow Short Description: Python library for converting complex datatypes to and from primitive types Upstream URL: http://marshmallow.readthedocs.org/ Owners: msuchy Branches: f21 f22 f23 InitialCC:
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? |fedora-cvs+
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #19 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=1219288 Bug 1219288 depends on bug 1260575, which changed state.
Bug 1260575 Summary: Review Request: python-ordered-set - A Custom MutableSet that remembers its order https://bugzilla.redhat.com/show_bug.cgi?id=1260575
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |NEXTRELEASE
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #20 from Fedora Update System updates@fedoraproject.org --- python-marshmallow-2.0.0-0.6.gita8b3385.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-16301
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #21 from Fedora Update System updates@fedoraproject.org --- python-marshmallow-2.0.0-0.6.gita8b3385.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with $ su -c 'dnf --enablerepo=updates-testing update python-marshmallow' You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-16301
https://bugzilla.redhat.com/show_bug.cgi?id=1219288 Bug 1219288 depends on bug 1260575, which changed state.
Bug 1260575 Summary: Review Request: python-ordered-set - A Custom MutableSet that remembers its order https://bugzilla.redhat.com/show_bug.cgi?id=1260575
What |Removed |Added ---------------------------------------------------------------------------- Status|CLOSED |ON_QA Resolution|NEXTRELEASE |---
https://bugzilla.redhat.com/show_bug.cgi?id=1219288 Bug 1219288 depends on bug 1260575, which changed state.
Bug 1260575 Summary: Review Request: python-ordered-set - A Custom MutableSet that remembers its order https://bugzilla.redhat.com/show_bug.cgi?id=1260575
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
--- Comment #22 from Fedora Update System updates@fedoraproject.org --- python-marshmallow-2.0.0-0.6.gita8b3385.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1219288
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed| |2015-10-31 22:49:01
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla