Bug 177825
Summary: | Review Request: torsmo - TyopoytaORvelo System MOnitor | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Andreas Bierfert <andreas.bierfert> |
Component: | Package Review | Assignee: | Chris Chabot <chabotc> |
Status: | CLOSED RAWHIDE | QA Contact: | David Lawrence <dkl> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | chabotc, fedora-extras-list |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-01-18 15:02:38 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: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Andreas Bierfert
2006-01-15 00:24:44 UTC
I'll pick this one up. Changing to FE-REVIEW. First few issues: - No icon in the gnome menu visible for the torsmo entry (in system tools) - It would be very usefull if torsmorc.sample was included in the %doc section, without it no one would know how to configure it.. - I'm building on FC5, so build failed because all the modular-x sections were commented out. I think it would be future looking of you used a if %{?dist} = 'fc5' kind of construction, or define %modular_x = (%{?dist} == 'fc5') and %if %{modular_x} etc .. If you need some help on this i could look up the specifics. However guideline is that any package submited will hit devel first so it should atleast build on fc5/devel. :) Thanks... Is not having an icon an issue now? Some of my packages don't have icons where there are no pixmaps provided. You are right about a sample rc ^^ I will include it. I always keep two different specs for old xorg and modular x. I will upload them with the next package release. Not having an icon is not a reason to be blocked from acceptance, but for the end user experiance its much better to have included one. So if its available (or can be made) please do, otherwise its not a blocker :-) I'll be doing test build and mock build on FC5, and the intial cvs import will by definition be for devel too, so please make that the primary one, in CVS when it gets branched you can then update for other branches with different dependencies. (In reply to comment #2) > I always keep two different specs for old xorg and modular x. I will upload them > with the next package release. Hi Andreas, i'm still hopefully waiting for new srpm's to test and do the formal review list on. No rush of cource but just so you know my status :-) Had a lot of stuff to do at university will get a new srpm out tonight... promise :) Late but still today :) http://fedora.lowlatency.de/review/torsmo-0.18-3.src.rpm http://fedora.lowlatency.de/review/torsmo.spec it is still missing the icon... do you have something in mind or should we leave it for now? Better late then never right? :-) Changelog format currently is: * Tue Jan 17 2006 Andreas Bierfert <andreas.bierfert[AT]lowlatency.de> 0.18-3 But should be: * Tue Jan 17 2006 Andreas Bierfert <andreas.bierfert[AT]lowlatency.de> - 0.18-3 Which makes rpmlint complain: W: torsmo no-version-in-last-changelog sample torsmo.rc is now included, i'm sure this will help users a lot :-) Might be worth adding a quick note in the description to check out the torsmorc.sample file in the doc dir? (just so they know how to find it, very much optional, just an idea) Whole torsmo project is missing an icon, so i guess its ok to leave it out :-) Formal review list: MUST review items: - Builds cleanly on FC5 devel. - rpmlint's only output is a version/changelog warning (see above) - Source included matches upsteam source (md5sum) - Package name meets guidelines - spec file name is in %{name}.spec format - Licence (BSD) is fedora extra's compatible & is included in spec - Spec file is in (american) english - Does not list buildrequires that are excepted in the package guidelines - All build dependencies are listed - No ldconfig needed - All files have proper permissions - Package is not relocatable - No duplicate files in %files section - No missing files in %files section - Has a proper %clean section with rm -rf $RPM_BUILD_ROOT - Uses macro's described in PackagingGuidelines - No entries in %doc that are required for standard program operation - No -devel package needed - No directory-ownerships needed - Has propper desktop file / -installation and desktop-file-utils BR Should items: - Includes upstream licence file (COPYING) - No insane scriplets - No unnescesarry requires - Mock builds cleanly on fc-devel-i386 FE-APPROVED but please do fix the changelog entries before commiting the srpm to cvs :-) ps the line wrapping made the version in changelog note come out all wrong. Changelog entries should be in the following format: * Date Who <email> - maj.min-rel How about this: Torsmo (TyopoytaORvelo System MOnitor) is a system monitor that sits in the corner of your desktop. Torsmo can show various information about your system and its peripherals. Torsmo is very light and customizable, and it renders text on the root window. For a sample configuration take a look torsmorc.sample. As to the changelog format: putting maj.min-rel in the next line was accepted for all my other packages because of my long name email combination. If you don't mind I will leave it as is :) Thanks for taking your time to review this. Desciption sounds good to me, i'm sure it'll help some users find their way :-) As for version line, well some external programs that depend on sane version lines might dislike it, but i guess if its already accepted policy due to your long name/email then who am i to argue :-) Ps pls don't forget to assign the bug to me on closing to NEXTRELEASE (after make tag/build) Thanks :) imported but buildsys is not working at... will close the bug once it is build for devel Please don't forget to assign it to yourself when you approve a package review. |