[Bug 823886] Review Request: libusbx - Library for accessing USB devices from userspace

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=823886

Oron Peled <oron@xxxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |fedora-review+

--- Comment #9 from Oron Peled <oron@xxxxxxxxxxxx> ---
> > [?]: MUST Changelog in prescribed format.
> > * The .spec file Changelog entry is OK.
> > 
> > * However, upstream ChangeLog points to git:
> >   - We cannot grab this during build (no network operations)
> >   - Upstream is correct (IMO), not maintaining a separate ChangeLog
> >   - The correct way is for upstream to generate it in a 'dist-hook'
> >     and add it to EXTRA_DIST. This way, tarball releases would
> >     contain a bundled and up to date ChangeLog as they should.
> >   - OTOH, upstream NEWS file looks very much like a changelog... hmmm...
> 
> Yes NEWS is the human readable form of the ChangeLog, iow it summarizes
> all the important changes while leaving out things like formatting cleanups :)
> So I believe that just shipping NEWS is the right thing to do, this provides
> the right amount of information for users / developers using libusbx, rather
> then people actually developping libusbx itself. And the latter catagory
> can simply look at the git history.

Agreed.

> > Problematic Obsoletes?
> >  * Many existing packages depends on either libusb, libusb1:
> >      [xxxx@rawhide ~]$ repoquery --whatrequires libusb1 | wc -l
> >      53
> >      [xxxx@rawhide ~]$ repoquery --whatrequires libusb | wc -l
> >      86
> > 
> 
> Right, notice that this package does not touch libusb-0.1 (which we package
> as libusb. It "only" replaces libusb-1.0 (which we package as libusb1).
>> >  * Upstream do not hint in the provided README, etc that this
> >    is a fork of libusb1 and plug-in compatible with libusb1=<version>:
> >    - It would be easier to track this if they did.
> > 
> >    - You mention this in the .spec changelog. I think this information
> >      is important enough to warrant a paragraph in %description
> >      (so people comparing their installed software with some upstream
> >      source can have a clue)
> 
> Good idea, I've extended the %description to include the relevant information

+1

> >    - Failing to notice your changelog remark, I googled for libusb1,libusbx
> >      and hit your mail from yesterday. Maybe (an edited) version of this
> >      may be added to %doc as README.Fedora
> 
> I've opted to instead add the relevant bits to %description, as I would like
> the stay as close to upstream as possible and not add any Fedora specific
> patches / README files.

+1

> >  * OK, assuming full ABI compatiblity (of current versions) there are no
> >    problematic obsoletes, just my problematic understanding of the situation.
> 
> Yes libusbx is fully ABI and API compatible with libusb-1.0.x (upto 1.0.9, which
> is the latest release.
> 
> > OK. Replacing userspace with user-space would drop some of the
> > spurious spelling errors (personal taste, could be ignored)
> 
> I've opted to instead just drop userspace making the Summary:
> Library for accessing USB devices
> 
> "Library" already makes it clear this is for userspace apps :)

+1

> > Yes, but the URL tag seems dubious:
> >  - Shouldn't it be:
> >       http://sourceforge.net/projects/libusbx
> 
> Right, I somehow got it wrong, fixed.

It now points to project wiki -- good.

> > [x]: SHOULD %check is present and all tests pass.
> > 
> > It would be nice if some of the examples would actually be
> > used for 'make check'. For example the listdevs.c is probabely
> > usefull to test compile+link+run. This should probably work
> > even within mock (or Debian pbuilder) where actuall USB devices
> > are not accessible.
> 
> Good idea, I've instead added --enable-examples-build to %configure, which does
> the same but then as part of %build

OK.


Package APPROVED

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]