Spec URL: http://matrix.senecac.on.ca/~aadavis1/packaging/xs-activation.spec SRPM URL: http://matrix.senecac.on.ca/~aadavis1/packaging/xs-activation-0.3.14.gd2a3727-2.fc17.src.rpm Description: Hi! I just finished packaging up xs-activation, and I would appreciate a review so that I can get it into fedora extras. Fedora Account System Username:aadavis1
Hi alex, this package was already submitted and refuse, please look at https://bugzilla.redhat.com/show_bug.cgi?id=879568 for the reason ( and a few notes on the previously exposed spec ) *** This bug has been marked as a duplicate of bug 879568 ***
Oups, wrong bug, sorry
So, I assume you do not have yet a sponsor, I can review and help you, but you will need to find one ( following https://fedoraproject.org/wiki/Package_Review_Process ) to sponsor if your task is to have them in Fedora. I applied for sponsorship 1 hour ago, but I am not guaranteed to become one, so better try to find one ( as i said to your comrade, I would suggest to get in touch with ctyler ). In the mean time, i will review this package.
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.
(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?
For package, if you want to do x86_64, you build on x86_64 host, and the same for i386. Once you have made correction, just give the url to the new version of the spec file and srpm.
Spec URL: http://matrix.senecac.on.ca/~aadavis1/packaging/xs-activation.spec SRPM URL: http://matrix.senecac.on.ca/~aadavis1/packaging/xs-activation-0.3.14.gd2a3727-2.fc17.src.rpm Description: Hi! I just finished packaging up xs-activation, and I would appreciate a review so that I can get it into fedora extras. correction have been made, I just comment (#) the places where a few things are not needed and update the changelog. Fedora Account System Username:aadavis1
hey I think uploading this in the fedora review will mark it as a duplicate so I post the links above. it the same source but file is uploaded. just waiting for a sponsor
Spec URL: http://matrix.senecac.on.ca/~aadavis1/packaging/xs-activation.spec SRPM URL: http://matrix.senecac.on.ca/~aadavis1/packaging/xs-activation-0.3.14.gd2a3727-2.fc17.src.rpm Description: Hi, I have rebuild this package to meet Fedora Packaging Guideline.I would appreciate your feedback and review. Fedora Account System Username: aadavis1
This is an automatic check from review-stats script. This review request ticket hasn't been updated for some time. We're sorry it is taking so long. If you're still interested in packaging this software into Fedora repositories, please respond to this comment clearing the NEEDINFO flag. You may want to update the specfile and the src.rpm to the latest version available and to propose a review swap on Fedora devel mailing list to increase chances to have your package reviewed. If this is your first package and you need a sponsor, you may want to post some informal reviews. Read more at https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group. Without any reply, this request will shortly be considered abandoned and will be closed. Thank you for your patience.
This is an automatic action taken by review-stats script. The ticket submitter failed to clear the NEEDINFO flag in a month. As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews we consider this ticket as DEADREVIEW and proceed to close it.