Bug 177825

Summary: Review Request: torsmo - TyopoytaORvelo System MOnitor
Product: [Fedora] Fedora Reporter: Andreas Bierfert <andreas.bierfert>
Component: Package ReviewAssignee: Chris Chabot <chabotc>
Status: CLOSED RAWHIDE QA Contact: David Lawrence <dkl>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: 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
Spec Name or Url: http://fedora.lowlatency.de/review/torsmo.spec
SRPM Name or Url: http://fedora.lowlatency.de/review/torsmo-0.18-2.src.rpm

Description:
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.

Comment 1 Chris Chabot 2006-01-15 20:48:22 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.



Comment 2 Andreas Bierfert 2006-01-15 21:33:40 UTC
:) 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.

Comment 3 Chris Chabot 2006-01-15 21:39:02 UTC
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.


Comment 4 Chris Chabot 2006-01-17 10:41:23 UTC
(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 :-)

Comment 5 Andreas Bierfert 2006-01-17 10:47:09 UTC
Had a lot of stuff to do at university will get a new srpm out tonight... promise :)

Comment 6 Andreas Bierfert 2006-01-17 21:44:58 UTC
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?

Comment 7 Chris Chabot 2006-01-17 21:58:28 UTC
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 :-)


Comment 8 Chris Chabot 2006-01-17 22:01:15 UTC
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


Comment 9 Andreas Bierfert 2006-01-17 22:10:18 UTC
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.

Comment 10 Chris Chabot 2006-01-17 22:15:14 UTC
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)

Comment 11 Andreas Bierfert 2006-01-17 22:34:34 UTC
Thanks :) imported but buildsys is not working at... will close the bug once it
is build for devel

Comment 12 Warren Togami 2006-01-18 15:02:38 UTC
Please don't forget to assign it to yourself when you approve a package review.