Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=879749 --- Comment #5 from Alex <aadavis1@xxxxxxxxxxxxxxxxxxx> --- (In reply to comment #4) > A few notes as part of the review : > > 1) Packager tag should not be used > https://fedoraproject.org/wiki/Packaging:Guidelines#Tags > > > 2) I do not think %post should be kept, as people may not read it, and that > it doesn't help much. I think there is even a policy to say that %post > should be silent. > > 3) > %install > echo "hello" > #rm -rf $RPM_BUILD_ROOT > pwd > ls > make DESTDIR=$RPM_BUILD_ROOT PYTHON_SITELIB=%{python_sitelib} install > > no need for echo, pwd, ls, as this is likely just for debugging. > > 4) having /library is forbidden in Fedora : > https://fedoraproject.org/wiki/Packaging:Guidelines#Filesystem_Layout > > We cannot create arbitrary top level directory. So you should see with > upstream to change this. > > 5) the changelog entry should be more descriptive > https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs > ( ie, explain what you changed before the previous version and this one > > 6) BuildArch: x86_64 > > Why limit to x86_64 ? > > > 7) THis one is subtle. > %{_sysconfdir}/sysconfig/olpc-scripts/setup.d/* > > If you install xs-activation, and remove it, as the directory > /etc/sysconfig/olpc-scripts/ is not listed in %files, it would not be > removed, and so this would be a leftover. We try to avoid that. See > https://fedoraproject.org/wiki/Packaging:UnownedDirectories for details > > 8) > BuildRequires: python-devel > > you need to explin if this is python2 or python3 > https://fedoraproject.org/wiki/Packaging:Python#BuildRequires ( otherwise, > this may break the day we switch to python3, so we try to be proactive and > prevent the issue before it happens ) > > 9) > Requires: bash > Requires: python > > Bash is preinstalled, and I think python will be automatically detected ( > ie, rpm will add the requires by itself ) > > 10) > Requires: usbmount > > usbmount is not in Fedora, so the package need to be added. > > 11) > %{!?python_sitelib: %global python_sitelib %(%{__python} -c "from > distutils.sysconfig import get_python_lib; print get_python_lib()")} > > not sure if that's needed anymore, since all supported Fedora should already > have the macro defined > > https://fedoraproject.org/wiki/Packaging:Python#Macros > > 12) the description is rather terse, and could IMHO be improved. > > 13) I think a better url would be http://wiki.laptop.org/go/XS-activation > > Do not hesitate to contact me ( either misc, on irc.freenode.net ), or ask > question in this bug if there is something unclear. thanks. for the x86_64 I use this because I was to do a x86_64 or i386 build on the package. I thought if noarch build was removed I will get an x86_64. I made changes to the correction should I resubmit the review? -- You are receiving this mail because: You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review