Bug 461571 - Review Request: sugar-log -- Log activity for sugar
Summary: Review Request: sugar-log -- Log activity for sugar
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jeremy Katz
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-09-09 08:31 UTC by Simon Schampijer
Modified: 2010-06-18 15:51 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-09-18 17:53:27 UTC
Type: ---
Embargoed:
katzj: fedora-review+


Attachments (Terms of Use)
SPEC URL (updated) (44 bytes, text/plain)
2008-09-09 08:31 UTC, Simon Schampijer
no flags Details
The source rpm (57 bytes, text/plain)
2008-09-12 12:31 UTC, Simon Schampijer
no flags Details
new SRPM (57 bytes, text/plain)
2008-09-13 10:44 UTC, Simon Schampijer
no flags Details

Description Simon Schampijer 2008-09-09 08:31:53 UTC
Created attachment 316157 [details]
SPEC URL (updated)

Log activity for sugar

Comment 1 Jeremy Katz 2008-09-09 19:56:25 UTC
A couple of things from a quick look
1) Need src.rpm link so that source can be verified against upstream
2) Shouldn't need gettext buildrequires anymore as I added the requires to sugar-toolkit so that it isn't needed in every package

Comment 2 Simon Schampijer 2008-09-12 12:31:18 UTC
Created attachment 316565 [details]
The source rpm

Comment 3 Simon Schampijer 2008-09-12 12:33:27 UTC
Jeremy, thanks for the review

1) i added the src.rpm
2) removed the gettext buildrequires
3) removed the psuedo.po check since we do remove it in the upcoming sugar-toolkit rpm

Comment 4 Jeremy Katz 2008-09-12 13:33:22 UTC
Okay, going through to do a more complete review.

* NEEDSWORK: The URL given for the tarball doesn't exist, it looks like Log-15 never got uploaded?  So can't verify against upstream source
* FYI: Please bump the release in the future when making changes, even as the result of review comments 
* FYI: Please request that upstream include a version of the GPL as COPYING within their source tarball


Builds fine in mock, rpmlint output has a few things which need fixing

* sugar-log.noarch: E: zero-length /usr/share/sugar/activities/Log.activity/README
sugar-log.noarch: E: zero-length /usr/share/doc/sugar-log-15/README

If it's empty, it's probably not worth including

* sugar-log.noarch: E: non-executable-script /usr/share/sugar/activities/Log.activity/logviewer.py 0644
sugar-log.noarch: E: non-executable-script /usr/share/sugar/activities/Log.activity/logcollect.py 0644

These should have the #!/usr/bin/python removed from the top if they're not meant to be directly executed.

* sugar-log.noarch: E: description-line-too-long Log is an activity for developers to examine the log files that are generated by

The description lines are supposed to be limited to 72 (iirc) characters

* sugar-log.noarch: W: non-standard-group Sugar/Activities

There's a bug tracking adding this as acceptable, so fine

* sugar-log.noarch: E: incorrect-locale-subdir /usr/share/locale/pseudo/LC_MESSAGES/org.laptop.Log.mo

If this will be getting fixed up in a future sugar-toolkit, then I'm fine with leaving the problem for now and it'll just get fixed up as things rebuild

Comment 5 Simon Schampijer 2008-09-13 10:42:08 UTC
(In reply to comment #4)
> Okay, going through to do a more complete review.
> 
> * NEEDSWORK: The URL given for the tarball doesn't exist, it looks like Log-15
> never got uploaded?  So can't verify against upstream source

Sorry they are uploaded now.

> * FYI: Please bump the release in the future when making changes, even as the
> result of review comments 

Sure.

> * FYI: Please request that upstream include a version of the GPL as COPYING
> within their source tarball

Done. 

> Builds fine in mock, rpmlint output has a few things which need fixing
> 
> * sugar-log.noarch: E: zero-length

Done.

> /usr/share/sugar/activities/Log.activity/README
> sugar-log.noarch: E: zero-length /usr/share/doc/sugar-log-15/README
> 
> If it's empty, it's probably not worth including

Removed from the sources.

> * sugar-log.noarch: E: non-executable-script
> /usr/share/sugar/activities/Log.activity/logviewer.py 0644
> sugar-log.noarch: E: non-executable-script
> /usr/share/sugar/activities/Log.activity/logcollect.py 0644
> These should have the #!/usr/bin/python removed from the top if they're not
> meant to be directly executed.

Fixed.
 
> * sugar-log.noarch: E: description-line-too-long Log is an activity for
> developers to examine the log files that are generated by

Done.
 
> The description lines are supposed to be limited to 72 (iirc) characters
> 
> * sugar-log.noarch: W: non-standard-group Sugar/Activities
> 
> There's a bug tracking adding this as acceptable, so fine
> 
> * sugar-log.noarch: E: incorrect-locale-subdir
> /usr/share/locale/pseudo/LC_MESSAGES/org.laptop.Log.mo
> 
> If this will be getting fixed up in a future sugar-toolkit, then I'm fine with
> leaving the problem for now and it'll just get fixed up as things rebuild

Yeah we just wait for another fix to get in.

Comment 6 Simon Schampijer 2008-09-13 10:44:56 UTC
Created attachment 316664 [details]
new SRPM

Comment 7 Rakesh Pandit 2008-09-13 10:47:04 UTC
Changed status from NEW to ASSIGNED

Comment 8 Jeremy Katz 2008-09-14 19:19:19 UTC
Great, looks good.

APPROVED.

Comment 9 Huzaifa S. Sidhpurwala 2008-09-17 03:11:59 UTC
Simon, 
Can you please add the CVS request in the specified format please.
It helps to cvs admin to determine a lot of things

---- EXAMPLE ----

New Package CVS Request
=======================
Package Name: foo
Short Description: foo bar foo
Owners: bar
Branches: F-8 F-9
InitialCC:

Comment 10 Simon Schampijer 2008-09-17 07:02:17 UTC
Sorry - missed this :/

New Package CVS Request
=======================
Package Name: sugar-log
Short Description: Tool to examine sugar log files 
Owners: erikos
Branches: F-9
InitialCC: mpg

Thanks!

Comment 11 Kevin Fenzi 2008-09-17 19:00:54 UTC
cvs done.

Comment 12 Jeremy Katz 2008-09-18 17:53:27 UTC
And this is built

Comment 13 Peter Robinson 2010-06-10 20:50:00 UTC
Package Change Request
======================
Package Name: sugar-log
New Branches: EL-6
Owners: pbrobinson sdz

Comment 14 Kevin Fenzi 2010-06-11 04:37:00 UTC
Have you checked with erikos to see if he would like to maintain in EPEL?


Note You need to log in before you can comment on or make changes to this bug.