[Bug 879749] Review Request: xs-activation - OLPC XS Activation Server

2012-12-04 Thread bugzilla
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=879749

Alex  changed:

   What|Removed |Added

  Flags||needinfo?(m...@zarb.org)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 879749] Review Request: xs-activation - OLPC XS Activation Server

2012-12-01 Thread bugzilla
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=879749

--- Comment #10 from Alex  ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 879749] Review Request: xs-activation - OLPC XS Activation Server

2012-11-26 Thread bugzilla
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=879749

--- Comment #9 from Alex  ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 879749] Review Request: xs-activation - OLPC XS Activation Server

2012-11-26 Thread bugzilla
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=879749

--- Comment #8 from Alex  ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 879749] Review Request: xs-activation - OLPC XS Activation Server

2012-11-25 Thread bugzilla
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=879749

--- Comment #7 from Alex  ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 879749] Review Request: xs-activation - OLPC XS Activation Server

2012-11-25 Thread bugzilla
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=879749

--- Comment #6 from Michael Scherer  ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 879749] Review Request: xs-activation - OLPC XS Activation Server

2012-11-25 Thread bugzilla
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=879749

--- Comment #5 from Alex  ---
(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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 879749] Review Request: xs-activation - OLPC XS Activation Server

2012-11-24 Thread bugzilla
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=879749

Michael Scherer  changed:

   What|Removed |Added

 Depends On||879752

--- Comment #4 from Michael Scherer  ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 879749] Review Request: xs-activation - OLPC XS Activation Server

2012-11-24 Thread bugzilla
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=879749

--- Comment #3 from Michael Scherer  ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 879749] Review Request: xs-activation - OLPC XS Activation Server

2012-11-24 Thread bugzilla
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=879749

Michael Scherer  changed:

   What|Removed |Added

 Status|ASSIGNED|NEW

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 879749] Review Request: xs-activation - OLPC XS Activation Server

2012-11-24 Thread bugzilla
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=879749

Michael Scherer  changed:

   What|Removed |Added

 Status|CLOSED  |ASSIGNED
 Resolution|DUPLICATE   |---
   Keywords||Reopened

--- Comment #2 from Michael Scherer  ---
Oups, wrong bug, sorry

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 879749] Review Request: xs-activation - OLPC XS Activation Server

2012-11-24 Thread bugzilla
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=879749

Michael Scherer  changed:

   What|Removed |Added

 Status|NEW |CLOSED
 Resolution|--- |DUPLICATE
Last Closed||2012-11-24 08:43:06

--- Comment #1 from Michael Scherer  ---
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 ***

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 879749] Review Request: xs-activation - OLPC XS Activation Server

2012-11-24 Thread bugzilla
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=879749

Michael Scherer  changed:

   What|Removed |Added

 CC||m...@zarb.org
 Blocks||177841 (FE-NEEDSPONSOR)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review