Bug 461571
| Summary: | Review Request: sugar-log -- Log activity for sugar | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Simon Schampijer <simon> | ||||||||
| Component: | Package Review | Assignee: | Jeremy Katz <katzj> | ||||||||
| Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||||
| Severity: | medium | Docs Contact: | |||||||||
| Priority: | medium | ||||||||||
| Version: | rawhide | CC: | fedora-package-review, huzaifas, katzj, notting, pbrobinson | ||||||||
| Target Milestone: | --- | Flags: | katzj:
fedora-review+
|
||||||||
| Target Release: | --- | ||||||||||
| Hardware: | All | ||||||||||
| OS: | Linux | ||||||||||
| Whiteboard: | |||||||||||
| Fixed In Version: | Doc Type: | Bug Fix | |||||||||
| Doc Text: | Story Points: | --- | |||||||||
| Clone Of: | Environment: | ||||||||||
| Last Closed: | 2008-09-18 17:53:27 UTC | Type: | --- | ||||||||
| Regression: | --- | Mount Type: | --- | ||||||||
| Documentation: | --- | CRM: | |||||||||
| Verified Versions: | Category: | --- | |||||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||||
| Embargoed: | |||||||||||
| Attachments: |
|
||||||||||
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 Created attachment 316565 [details]
The source rpm
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 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 (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. Created attachment 316664 [details]
new SRPM
Changed status from NEW to ASSIGNED Great, looks good. APPROVED. 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: 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! cvs done. And this is built Package Change Request ====================== Package Name: sugar-log New Branches: EL-6 Owners: pbrobinson sdz Have you checked with erikos to see if he would like to maintain in EPEL? |
Created attachment 316157 [details] SPEC URL (updated) Log activity for sugar