https://bugzilla.redhat.com/show_bug.cgi?id=1240116 --- Comment #21 from Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> --- Thanks for taking on the review! (In reply to William Moreno from comment #20) > Package Review > ============== > > 1. The doc and common subpackage both ship the license text, I will recomend > to include the license text in the common packages and include a weak > dependencie in doc: > > Suggest: python-music21-common I don't think this is allowed. The license file MUST be installed with any subcombination of packages. > Also use weak dependencies in python2 and 3 subpackage to suggest the doc > subpackage. Good idea, done. > 2. The common subpackage include the python2.7 files than are linked to the > python3 subpackage. As part of the move of Fedora to python3 by default I > will recomend to split the common subpackage to ship the python3.5 files. > Currently the common subpackage will requires python2 as the owner of the > /usr/lib/python2.7/site-packages/ directory so python3-music21 will require > python2 to install python-music21-common. I moved them. > There are many warning about symlinks in both packages, also please check > this issue. I think that the symlinks are fine. rpmlint does not understand symlinks. > The best aproach to this common content is to place it in /usr/share but > this will require some mayor patching to the code and this will require to > work with upstream. I could move the files in common to /usr/share and provide symlinks from both subpackages. I don't know if this would be better. > 3. Include in python2 and 3 %files > > %exclude %{python2_sitelib}/music21/LICENSE > The license file is in the common subpackage. Done. > 3. Consider to include the test directory in both packages into a devel > subpackage (python2-name-devel and python3-name-devel), most user will not > run the test locally and this will let then get a smaller package to > download. > > 4. Also both packages have a demos directory than do not look to be necesary > in run time and can be packaged as subpackages, you can use weak depencies > to suggest the user to install the demos subpackages. I don't think splitting it up like this makes sense. It is essentially a user program (using python as its interface), not a library that is installed as a dependency for other things. Demos can be used as documentation and are useful when using the package. They are not too big too. > 5. Package is not known to require an ExcludeArch tag. Builds fails in > secondary arches, but looks like there is a issue with the packaging root. Hm, I'll try to rebuild. > RPMLint: > 1: python2-music21.noarch: E: explicit-lib-dependency python-matplotlib Not > need work, it is a rpmlint issue I actually submitted a PR for that, but it doesn't seem like it will be merged: https://github.com/rpm-software-management/rpmlint/pull/58 > 2: python3-music21.noarch: W: dangling-symlink > /usr/lib/python3.4/site-packages/music21/corpus/cpebach > /usr/lib/python2.7/site-packages/music21/corpus/cpebach If you change the > common to python3 files this warning will be fixed but you will need to > check if the link in python2 works fine. > 3: python3-music21.noarch: E: non-executable-script > /usr/lib/python3.4/site-packages/music21/alpha/trecento/exceldiff.py 644 > Sedout the shebang of this files Done. > 4: python-music21-common.noarch: E: incorrect-fsf-address > /usr/lib/python2.7/site-packages/music21/corpus/josquin/laPlusDesPlus.abc > Request upstream to fix it or propose a patch to update it. https://github.com/cuthbertLab/music21/pull/122 > 5: python-music21-doc.noarch: W: wrong-file-end-of-line-encoding > /usr/share/doc/python-music21/html/_sources/tutorials/advancedGraphing.txt > See: https://fedoraproject.org/wiki/Packaging_tricks#Remove_DOS_line_endings Done. I need to update to 2.2.1. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review