Bug 177825 - Review Request: torsmo - TyopoytaORvelo System MOnitor
Review Request: torsmo - TyopoytaORvelo System MOnitor
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Chris Chabot
David Lawrence
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-01-14 19:24 EST by Andreas Bierfert
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-01-18 10:02:38 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Andreas Bierfert 2006-01-14 19:24:44 EST
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 15:48:22 EST
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 16:33:40 EST
:) 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 16:39:02 EST
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 05:41:23 EST
(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 05:47:09 EST
Had a lot of stuff to do at university will get a new srpm out tonight... promise :)
Comment 6 Andreas Bierfert 2006-01-17 16:44:58 EST
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 16:58:28 EST
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 17:01:15 EST
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 17:10:18 EST
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 17:15:14 EST
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 17:34:34 EST
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 10:02:38 EST
Please don't forget to assign it to yourself when you approve a package review.

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