Bug 225242 - Merge Review: alsa-utils
Merge Review: alsa-utils
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Matthias Saou
Fedora Package Reviews List
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-29 15:59 EST by Nobody's working on this, feel free to take it
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

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


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-29 15:59:59 EST
Fedora Merge Review: alsa-utils

http://cvs.fedora.redhat.com/viewcvs/devel/alsa-utils/
Comment 1 Matthias Saou 2007-08-31 08:21:52 EDT
Please find an updated spec file and a patch to the current spec file containing
some suggested changes and fixes :
http://thias.fedorapeople.org/merge-review/alsa-utils/

- Update License field.
- Mark udev rule as config.
- Use find_lang macro again to include translations (why was it removed?).
Comment 2 Matthias Saou 2007-09-12 07:16:59 EDT
Ping? It would be nice to get this package cleaned up for Fedora 8.
Comment 3 Martin Stransky 2007-09-20 11:18:01 EDT
added to rawhide.
Comment 4 Matthias Saou 2007-09-24 09:29:20 EDT
Additional cleanups :
- Pick either tabs or spaces for identing, but not both.
- Why is the -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 still needed? The
configure script only has a "--disable-largefile" option, so I'd assume it's
enabled by default. If it's still needed, a quick comment would be a good thing.
- The "/var/lib/alsa" line would be clearer as "%dir /var/lib/alsa/" since
nothing below it is included (only asound.state, but it's %ghost'ed).
Comment 5 Martin Stransky 2007-09-24 10:11:41 EDT
(In reply to comment #4)
> Additional cleanups :
> - Pick either tabs or spaces for identing, but not both.
> - Why is the -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 still needed? The
> configure script only has a "--disable-largefile" option, so I'd assume it's
> enabled by default. If it's still needed, a quick comment would be a good thing.

Aha. Unfortunately I have 64bit system so it's that always enabled there. I'll
try to check it on some i386.

> - The "/var/lib/alsa" line would be clearer as "%dir /var/lib/alsa/" since
> nothing below it is included (only asound.state, but it's %ghost'ed).

Feel free to fix it.
Comment 6 Matthias Saou 2007-09-25 05:01:47 EDT
(In reply to comment #5)
> (In reply to comment #4)
> > Additional cleanups :
> > - Pick either tabs or spaces for identing, but not both.
> > - Why is the -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 still needed? The
> > configure script only has a "--disable-largefile" option, so I'd assume it's
> > enabled by default. If it's still needed, a quick comment would be a good thing.
> 
> Aha. Unfortunately I have 64bit system so it's that always enabled there. I'll
> try to check it on some i386.

I've compared builds for i386 and for x86_64, and configure displays this for both :
 checking for special C compiler options needed for large files... no
 checking for _FILE_OFFSET_BITS value needed for large files... no
 checking for _LARGE_FILES value needed for large files... no
It's very likely that the override was needed at some point, but not any more it
seems.

> > - The "/var/lib/alsa" line would be clearer as "%dir /var/lib/alsa/" since
> > nothing below it is included (only asound.state, but it's %ghost'ed).
> 
> Feel free to fix it.

OK, I've done that, as well as changed the indentation to be the same as
alsa-lib.spec.

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