https://bugzilla.redhat.com/show_bug.cgi?id=1478231 --- Comment #6 from Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> --- Thanks you for taking the review. Being so through has been very helpful. I fixed most stuff (comments below), except some parts I disagree with. > Issues: > ======= > - All build dependencies are listed in BuildRequires, except for any that > are listed in the exceptions section of Packaging Guidelines. > Note: These BR are not needed: sed > See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 fedora-review is wrong here. If you follow the link it shows, actually gcc is *not* guaranteed to be present and the list of exceptions has been removed a while back. > [!]: License field in the package spec file matches the actual license. > Note: Checking patched sources after %prep for licenses. Licenses > found: "Apache (v2.0)", "Unknown or generated", "MIT/X11 (BSD like)", > "BSD (3 clause) LGPL", "BSD (unspecified)", "*No copyright* BSD > (unspecified)", "BSD (3 clause)", "LGPL (v2.1 or later)". 265 files > have unknown license. licensecheck.txt is attached. > Note: License tag of python*-conda should be "BSD and ASL 2.0 and > LGPLv2+ and MIT", and multiple licensing description should be provided. Good catch! I updated the License field and added a bunch of Provides: bundles(...) fields. I also looked into unbundling, but the "vendored" copies have different version then the ones in fedora, and have local modifications. So I think in this case it just isn't worth the risk of breaking stuff. > [!]: Package obeys FHS, except libexecdir and /usr/target. > Note: Fedora does not allow new directories directly under / or /usr > without FPC approval. > Note: /usr/condarc.d is provided. Right. I patched the python code to look in /usr/share/conda/condarc.d, and moved the file we create to that directory. > Note: %_bindir/{de,}activate scripts are used for being sourced, but not > executed. Placing them to %_datadir seems more proper. Even if they are sourced, $PATH is used, so it's more convenient to have them in /usr/bin ('source activate' works, and the full path would have to be given otherwise). > [?]: Package functions as described. > Note: Re-examine this after filesystem layout revised. > [!]: Description of activate > subpackage is bad. I fixed the description of activate subpackage to explain what it does. @Orion: I carried that package over from your spec file. Is there a particular reason to make this a seperate subpackage? > Descriptions of some subpackages are the same. I don't think there's any prohibition against having the same package desctiptions. With python2-/python3- packages it's extremely common to have the exact same text, and this doesn't lead to any confusion because the package name is enough to understand that one is for python2 and the other for python3. And I think it's pretty clear without stating it explicitly, that conda is the main package and python[23]-conda are the python modules backing it. [!]: Package consistently uses macros (instead of hard-coded directory names). Note: Many occurrences of "conda" can be replaced with %{name}. There's no reason to prefer %{name} to conda. It's longer, harder to type, and harder to read. There are two reasons when a macro should be used: - the string it replaces is long or awkward to type (e.g. because of weird casing or whatever), - the string varies, e.g. between architectures, or distro releases. For strings which are both shorter then the macro, and are unlikely to ever change, macros are just obfuscation. The guidelines talk about hardcoded directory names. E.g. %_libdir is necessary because it is /usr/lib64 sometimes, and /usr/lib othertimes. But most of the macro usage in spec files is pointless self-flagellation. ;) (In reply to Orion Poplawski from comment #3) > Created attachment 1315375 [details] > patch to try to support epel7 Added. (In reply to Elliott Sales de Andrade from comment #5) > Is there much need to package a Python 2 executable (on Fedora at least)? Python 2 versions create python2 environments. It's also possible to do it through other means (e.g. by calling 'python2 /usr/bin/conda create ...'), but having the versioned executables makes it easier. Spec URL: https://in.waw.pl/~zbyszek/fedora/conda.spec SRPM URL: https://in.waw.pl/~zbyszek/fedora/conda-4.3.24-1.fc27.src.rpm %changelog - Add all licenses to the License tag - Add Provides: bundled(...) for all the "vendored" dependencies - Update descriptions and simplify the spec file a bit - Move condarc.d directory under /usr/share/conda -- 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 To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
