Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 194563 - Review Request: conman - the console manager
Summary: Review Request: conman - the console manager
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Stone
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-06-14 05:00 UTC by Jarod Wilson
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-06-21 20:59:36 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Jarod Wilson 2006-06-14 05:00:48 UTC
Spec URL: http://wilsonet.com/packages/conman/conman.spec
SRPM URL: http://wilsonet.com/packages/conman/conman-0.1.9.1-1.src.rpm
Description: 
ConMan is a serial console management program designed to support a large
number of console devices and simultaneous users.  It currently supports
local serial devices and remote terminal servers (via the telnet protocol).
Its features include:

  - mapping symbolic names to console devices
  - logging all output from a console device to file
  - supporting monitor (R/O), interactive (R/W), and
    broadcast (W/O) modes of console access
  - allowing clients to join or steal console "write" privileges
  - executing Expect scripts across multiple consoles in parallel

Comment 1 Jarod Wilson 2006-06-14 15:38:42 UTC
The upstream initscript is an unholy mess that tries to support multiple
distributions, so I plan to create a stripped-down RH/FC-only version, but
otherwise, the package is ready for review.

Comment 2 Jarod Wilson 2006-06-14 17:21:58 UTC
Okay, new -2 build pushed up, contains a much cleaner initscript and also
properly sets up log directories and log rotation.

http://wilsonet.com/packages/conman/conman-0.1.9.1-2.src.rpm


Comment 3 Parag AN(पराग) 2006-06-18 12:21:16 UTC
Review for this package:-
MUST Items:
     - MUST: rpmlint shows no error 
     - MUST: The package is named according to the Package Naming Guidelines.
     - MUST: The spec file name matching the base package conman, in the format
conman.spec
      - MUST: This package meets the Packaging Guidelines.
      - MUST: The package is licensed with an open-source compatible license GPL.
      - MUST: The License field in the package conman.spec file is included in
tarball as COPYING.
      - MUST: The sources used to build the package matches the upstream source,
as provided in the spec URL. md5sum is correct.
      - MUST: This package owns all directories that it creates. 
      - MUST: This package did not contain any duplicate files in the %files
listing.
      - MUST: This package  have a %clean section, which contains %{__rm} -rf
%{buildroot}.
      - MUST: This package used macros.
      - MUST: Document files are NOT included like README.
     Package installed correctly the conman service

SHOULD items:-
        Upstream package should add README file

Comment 4 Parag AN(पराग) 2006-06-19 10:56:19 UTC
Above is Not an official review as I'm not yet sponsored

Comment 5 Jarod Wilson 2006-06-19 13:41:28 UTC
You don't have to be sponsored to do package reviews, being sponsored is only
necessary to submit and maintain your own packages in extras. Doing reviews
*before* getting sponsored is actually encouraged, as it helps potential
sponsors better evaluate your understanding of packaging.

As for the review, the only issue I see brought up is that there's no README,
which isn't an acceptance blocker. I'm a little unclear as to whether you're
saying that's the only thing missing or if you didn't see any documentation, but
all documentation included in the upstream tarball is included in the rpm
(except the INSTALL doc, which is irrelevant for an rpm).

So basically, as I understand it, you don't see any problems with the package
outside of the lack of an upstream README, and you actually *do* have the power
to approve the package. :)

Comment 6 Parag AN(पराग) 2006-06-19 13:51:29 UTC
you are right. Actually i thought some README file should be there that helps
end user to understand what package provides and is that require any dependency
to be installed prior.
Else package is Good.

Comment 7 Paul Howarth 2006-06-19 13:54:32 UTC
(In reply to comment #5)
> So basically, as I understand it, you don't see any problems with the package
> outside of the lack of an upstream README, and you actually *do* have the power
> to approve the package. :)

The Package Review Guidelines say:

  The primary Reviewer can be any current package owner, unless the Contributor
  is a first timer.

Somebody that is not sponsored cannot be a current package onwer, hence they
cannot be the *primary* reviewer, hence they cannot approve packages.

Parag is of course encouraged to review packages as part of the process of
getting sponsored, but formally reviewing and approving packages must be done by
an existing package owner.

Comment 8 Jarod Wilson 2006-06-19 14:13:59 UTC
(In reply to comment #7)
> The Package Review Guidelines say:
> 
>   The primary Reviewer can be any current package owner, unless the Contributor
>   is a first timer.
> 
> Somebody that is not sponsored cannot be a current package onwer, hence they
> cannot be the *primary* reviewer, hence they cannot approve packages.
> 
> Parag is of course encouraged to review packages as part of the process of
> getting sponsored, but formally reviewing and approving packages must be done by
> an existing package owner.


Ah, okay, thank you for the clarification, mistakenly made the leap from being
encouraged to review to being able to approve.

Comment 9 Christopher Stone 2006-06-20 23:00:42 UTC
- rpmlint output clean
- Package is named according to Package Naming Guidelines
- spec file matches base package %{name}
- package meets Packaging Guidelines
- package is licensed with open source compatible license
- license field matches actual license
- license text included in %doc
- spec file written in American english
- spec file is legible
- source package matches upstream
b47730a326376cf731c313900095449c  conman-0.1.9.1.tar.bz2
- package successfully compiles and builds on FC-5 x86_64
- All build dependencies are listed
- package does not use locales
- package does not contain any shared libraries
- package is non-relocatable
- package owns all directories it creates
- no duplicate files in %files
- file permissions set appropriately
- contains proper %clean section
- macro usage consistant
- package contains permissible content
- package does not contain large documentation
- %doc files do not affect runtime
- no headers or static libraries
- no pkgconfig files
- no .so or .la files
- no GUI or .desktop file needed
- package does not own files or directories owned by other packages

=== MUST ===
- Package must add "Requires: logrotate"
- service conman start says [ OK ] but there is no conmand process started

Comment 10 Jarod Wilson 2006-06-21 04:04:58 UTC
Okay, added 'Requires: logrotate' and figured out what was up w/the non-running conmand. The problem 
is that conmand exits cleanly if no CONSOLE devices are defined in /etc/conman.conf, meaning RETVAL 
was 0 (success), so I added a check to the initscript that will trigger a failure if none are present. I'll yell 
upstream and tell them this behavior is el stupido, but the initscript workaround should suffice for now.

http://wilsonet.com/packages/conman/conman.spec
http://wilsonet.com/packages/conman/conman-0.1.9.1-3.src.rpm

I'd do that ckfsv review for you now, but it looks like someone beat me to it... I owe ya one. :)

Comment 11 Christopher Stone 2006-06-21 07:33:07 UTC
All must items fixed. APPROVED.

Comment 12 Jarod Wilson 2006-06-21 14:13:04 UTC
Imported and built for devel, pending cvs branching for other builds.

Comment 13 Jarod Wilson 2006-06-21 20:59:36 UTC
Built for FC4 and 5, closing ticket.

Comment 14 Christian Iseli 2006-12-31 10:51:37 UTC
(In reply to comment #13)
> Built for FC4 and 5, closing ticket.

Please do not remove the FE-ACCEPT blocker.  Thanks.



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