Bug 330951 - Review Request: nbd - Network Block Device
Review Request: nbd - Network Block Device
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Warren Togami
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-13 16:19 EDT by Eric Harrison
Modified: 2014-01-24 07:40 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-10-15 15:26:35 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
wtogami: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Comment 1 Patrice Dumas 2007-10-13 17:24:55 EDT
If simple_test is shipped, it seems to me that the sources
for nbd-tester-client should also be shipped
(nbd-tester-client.c cliserv.h).

I think that the check could be added: 
%check
make check

Reading the README, it looks like some special devices should
be created. Should it be left to the user, will the devices be 
created on the fly, or should they be created in a post-script?
Comment 2 Patrice Dumas 2007-10-13 17:41:39 EDT
The license seems to me to be GPL+ 

The server and the client should certainly be in different
packages, especially since according to the README they
shouldn't be installed on the same computer.

You could consider adding %dist.

rpmlint relevant complaint:
nbd.i386: W: no-version-in-last-changelog

Comment 3 Warren Togami 2007-10-13 18:08:11 EDT
[builder2@newcaprica x86_64]$ rpmlint nbd-2.9.7-1.x86_64.rpm 
nbd.x86_64: W: spurious-executable-perm /usr/share/doc/nbd-2.9.7/simple_test
nbd.x86_64: W: no-version-in-last-changelog

1)
OK to ignore the first warning.

2)
Highly recommended that you include the V-R after your name in each changelog
entry.  It would look like this:

* Sat Oct 13 2007 Eric Harrison <eharrison@mesd.k12.or.us> 2.9.7-1
- update to 2.9.7

3) Change the license to "GPL+".

4) Add %{dist} to the end of your Release tag.

With these minor changes this package is APPROVED.
Comment 4 Patrice Dumas 2007-10-13 18:13:08 EDT
I made other comments above that are worth discussing
or taking into account. That way to handle reviews is 
just plain unacceptable.
Comment 5 Warren Togami 2007-10-13 18:42:48 EDT
> The server and the client should certainly be in different
> packages, especially since according to the README they
> shouldn't be installed on the same computer.

I disagree that it is necessary to split this package.  It would be overly
pedantic to do so.  It is certainly not harmful to have both installed on the
same system but to use only one or the other.

On a more general level, we really want to avoid gratuitous package splits, as
every additional package slows down every yum transaction.  This is a tiny and
simple package with no real benefit in splitting.

> Reading the README, it looks like some special devices should
> be created. Should it be left to the user, will the devices be 
> created on the fly, or should they be created in a post-script?

Please leave this to the user for now.  nbd can be used in many different ways.
 LTSP specifically creates devices on-the-fly for an arbitrary number of
clients, so it is wrong for the nbd base package to make any assumptions.
Comment 6 Warren Togami 2007-10-13 19:12:57 EDT
Going ahead with import and build of nbd because this is seemingly less
controversial than Bug #330921, and I have only a few hours remaining to train
Eric Harrison on using the fedora CVS and buildsys before I fly out.

If there are further concerns about the package we can address it after initial
import.  Eric does need practice on updating an existing package and requesting
builds.

Comment 7 Eric Harrison 2007-10-13 19:25:25 EDT
New Package CVS Request
=======================
Package Name: nbd
Short Description: Network Block Device user-space tools
Owners: eharrison@mesd.k12.or.us
Branches: F-7
InitialCC: 
Cvsextras Commits: yes
Comment 8 Patrice Dumas 2007-10-13 20:03:57 EDT
(In reply to comment #6)
> If there are further concerns about the package we can address it after initial
> import.  

Ok.

> Eric does need practice on updating an existing package and requesting
> builds.

That's not very right, since all the fedora contributors should
be treated equally, but not a big deal either.

(In reply to comment #5)
> > The server and the client should certainly be in different
> > packages, especially since according to the README they
> > shouldn't be installed on the same computer.
> 
> I disagree that it is necessary to split this package.  It would be overly
> pedantic to do so.  It is certainly not harmful to have both installed on the
> same system but to use only one or the other.
> 
> On a more general level, we really want to avoid gratuitous package splits, as
> every additional package slows down every yum transaction.  This is a tiny and
> simple package with no real benefit in splitting.

I didn't said it was a must. The yum transaction slowing
is not a good reason in my opinion, if this is a concern, 
then we won't be able to scale anyway.

However it is true that splitting a package like this one
has advantages and disadvantages:

Advantages:
* since only one of both should be installed, it reduces
  the size of an installation. Of course this is by a small
  amount, but for this specific software one could imagine 
  setups where size matters.
* having things that are not useful not installed makes
  administration simpler.

Disadvantages:
* more complexity at the packaging level
* 2 packages, so the user has to know that the package is 
  split (although the names should be pretty straightforward
  to find out)

It seems to me that from a user point of view, having a split
package is better in that case (and in all the client/server
cases, in my opinion). In the end, leaving it to the packager 
seems right to me, still this has to be discussed.


> Please leave this to the user for now.  nbd can be used in many different ways.
>  LTSP specifically creates devices on-the-fly for an arbitrary number of
> clients, so it is wrong for the nbd base package to make any assumptions.

Fine.
Comment 9 Kevin Fenzi 2007-10-14 15:43:58 EDT
This cvs request seems to already have been processed.
Please reset if you need further cvsadmin action.
Comment 10 Christopher Meng 2014-01-23 19:34:47 EST
Maintainer here.

Package Change Request
======================
Package Name: nbd
New Branches: epel7
Owners: cicku
Comment 11 Jon Ciesla 2014-01-24 07:40:45 EST
Git done (by process-git-requests).

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