https://bugzilla.redhat.com/show_bug.cgi?id=976886 --- Comment #3 from Marcin.Dulak@xxxxxxxxx --- Spec URL: http://marcindulak.fedorapeople.org/packages/python-ase/v02/python-ase.spec SRPM URL: http://marcindulak.fedorapeople.org/packages/python-ase/v02/python-ase-3.7.1.3184-2.fc20.src.rpm These are the changes: - added ase-gui.desktop, for the ag command. It refers to Icon=/usr/share/python-ase/doc/_static/ase.ico . Is it OK? - bug#976886#c2 fixed - spec uses a generated files.list with %dir, %lang and py* files (In reply to Björn Esser from comment #1) > Issues found, so far: > > * License-Tag is incorrect: > > MIT/X11 (BSD like) > ------------------ > python-ase-3.7.1.3184/ase/io/fortranfile.py > > ---> should be LGPLv2+ and MIT OK > > * Lots of sources and byte-compiled files end-up in > %{_datadir}/%{name}/doc/: > > ---> there shouldn't be any byte-compiled stuff in docs > they should be placed as %doc examples/, I think. Would it be enough to remove (and is it really necessary) the py{c,o} files? I need to give a bit of explanation. We use %{_datadir}/%{name}/doc/ to store raw documentation (*rst), example ASE python scripts (not part of the code), and some additional static files (for example the ase-gui icon is stored there). Doing: cd %{_datadir}/%{name}/doc/ sphinx-build . _build would build ASE html documentation - but that takes time (~10 min), and requires additional dependencies (povray). We choose %{_datadir}/%{name} in order to have a standard fallback for distutils data_files directory for the following reason: on Fedora docs are installed under %{_datadir}/doc/%{name}-%{version} but on Debian under (corresponding) %{_datadir}/doc/%{name}-doc. The %{_datadir}/%{name} directory exists on both distributions (but for different purposes). I can also see that numpy for example, which packages it's docs on Fedora under %{_datadir}/doc/%{name}-%{version}, also includes byte-compiled files: $ rpm -ql numpy | grep basics\.pyc > > * %defattr(-,root,root,-) is present: > > ---> not needed in all maintained F/EL branches > please remove OK > > * BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) > > ---> prefered, but not mandatory, BuildRoot-path is: > %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) OK, removed > > * macros in comments: > > python-ase.spec:98: W: macro-in-comment %{python_sitelib} > python-ase.spec:98: W: macro-in-comment %{upstream_name} > python-ase.spec:99: W: macro-in-comment %exclude > python-ase.spec:99: W: macro-in-comment %{python_sitelib} > python-ase.spec:99: W: macro-in-comment %{upstream_name} > 0 packages and 1 specfiles checked; 0 errors, 5 warnings. > > ---> use doubled % to comment out these > remove unneded non-explaining comments from spec OK, removed > > * PatchX: %{name}-%{version}... > > ---> name of patch should be static. those patches may apply to later > versions as well. > You possibly want propose your patches for inclusion in > upstream-source and provide a link to this in comments The 2 patches have been already applied upstream, and are needed only for the release being packaged here. The relevant commits upstream are documented in the spec. I prefer to keep the patches versioned, but can change them to static names if that's a strict requirement. My impression was always that a patch should contain version information if possible. I keep patches for some projects that won't accept my dirty changes for years, and having them versioned allows me to build old and new versions: i change the version of the patch applied instead of changing the patch itself. > > * you may wish to remove conditional around Version-tag by simply prefixing > value of %{?upstream_svn} with a dot [.] OK > > * there are no manpages for %{_bindir}/* > > ---> since you are affiliated with upstream you may want to create and > include them by upstream-source our developers have been talking already for several years about man pages, but no one will work on that in the near future > > * Is there a way to have this build with python3, too? ASE does not run with python3 yet. > > Please fix them and I'll take a more detailed review, then. thanks -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=w516j0xflb&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review