On 2/3/11 12:18 PM, "Shane Bryan" <[email protected]> wrote:

>On Thu,  3 Feb 2011 09:25:48 -0800 (PST)
>Mael Pouessel <[email protected]> wrote:
>
>> Hi,
>> Adding new package pulseaudio-settings-mfld in project
>> devel:multimedia. Please review and accept ASAP.
>> 
>> Justification for this new package:
>> 
>> New configuration package for pulseaudio in Medfield project
>> (BMC#13179)  : Build dependencies with pulseaudio and
>> pulseaudio-modules-mfld packages
>
><snip>
>
>> This is a NEW package in devel:multimedia project.
>> The files in the new package:
>> pulseaudio-settings-mfld/
>>   |__  pulseaudio-settings-mfld-1.0.tar.bz2
>>   |__  pulseaudio-settings-mfld.changes
>>   |__  pulseaudio-settings-mfld.spec
>
>No .yaml file?  I thought these were required now, with the exception
>being for those packages that were too complex, which this does not
>appear to be.

(Just fyi, no, .yaml is not required.  It's there for people who find it
useful, and for those who don't have much experience packaging.)

http://wiki.meego.com/Packaging/Guidelines#Writing_a_package_from_scratch


>
>> 
>> The content of the spec file, pulseaudio-settings-mfld.spec:
>> ===================================================================
>> # 
>> # Do NOT Edit the Auto-generated Part!
>> # Generated by: spectacle version 0.22git
>> # 
>> # >> macros
>> # << macros
>> 
>> Name:       pulseaudio-settings-mfld
>> Summary:    PulseAudio settings for Intel mfld
>
>Might be clearer to spell out "Medfield" instead of the abbreviation.
>
>> Version:    1.0
>> Release:    0
>> Group:      System/Libraries
>> License:    GPLv2
>> BuildArch:  noarch
>> Source0:    %{name}-%{version}.tar.bz2
>> BuildArch:  noarch
>
>Nitpick, why list BuildArch twice?
>
>Moreover, since this *is* a platform specific package (medfield),
>should it really be noarch?
>
>> 
>> %description
>> PulseAudio settings for Intel MFLD
>
>Would be clearer to spell out "Medfield" instead of the abbreviation.
>
><snip>
>
>> install -d $RPM_BUILD_ROOT/var/lib/pulse-nokia/
>> cp -a var/lib/pulse-nokia/algs/ $RPM_BUILD_ROOT/var/lib/pulse-nokia/
>> cp -a var/lib/pulse-nokia/modes/ $RPM_BUILD_ROOT/var/lib/pulse-nokia/
>
>Since this is a config package for Medfield (an Intel generic platform
>name) and not for any Nokia specific device, why are these being
>installed into a "nokia" named directory?  Shouldn't it be "pulse-mfld"
>instead?
>
>> # << install pre
>> 
>> # >> install post
>> # << install post
>> 
>> %files
>> %defattr(-,root,root,-)
>> # >> files
>> /usr/share/pulseaudio/alsa-mixer-mfld/*
>> 
>> 
>> /var/lib/pulse-nokia/algs/aep/*
>> /var/lib/pulse-nokia/algs/voice/*
>> 
>> #/var/lib/pulse-nokia/modes/a2dp/*
>> #/var/lib/pulse-nokia/modes/hf/*
>> /var/lib/pulse-nokia/modes/hp/*
>> /var/lib/pulse-nokia/modes/hs/*
>> #/var/lib/pulse-nokia/modes/hsp/*
>> /var/lib/pulse-nokia/modes/ihf/*
>
>Same here, why "pulse-nokia" instead of "pulse-mfld"?
>
>> 
>> %config /etc/skel/.pulse/default.pa
>> %config /etc/skel/.pulse/daemon.conf
>> %config /etc/skel/.asoundrc
>> 
>> %config /lib/udev/rules.d/91-nCDKsoundcards.rules
>
>This looks more like a *specific* device udev rule and not a platform
>generic one... why not make it "91-mfldsoundcards.rules"?  If it is
>device specific (which I think we're trying to avoid), then it ought to
>go in an "nCDK" specific package like "pulseaudio-settings-nCDK", not
>one that is supposedly Medfield generic, correct?
>
>Shane...
>_______________________________________________
>MeeGo-packaging mailing list
>[email protected]
>http://lists.meego.com/listinfo/meego-packaging

_______________________________________________
MeeGo-packaging mailing list
[email protected]
http://lists.meego.com/listinfo/meego-packaging

Reply via email to