[Bug 880763] Review Request: AudioCuesheetEditor

[bugzilla at redhat.com]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=880763

Michael Schwendt <mschwendt at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |177841 (FE-NEEDSPONSOR)

--- Comment #7 from Michael Schwendt <mschwendt at gmail.com> ---
Setting the FE-NEEDSPONSOR flag, as I've found other tickets from you where it
is still set, too.

Finding a potential sponsor is best done by being communicative about the
Packaging Guidelines and the Review Guidelines. Ask questions, acknowledge the
guidelines with an attempt at reviewing your own package in accordance with the
ReviewGuidelines page in the Wiki. Or try to review other packages.

[...]

> Summary:        Editor for Audio Cuesheets.

No big issue, but one day all summaries will not end with a dot. I would also
change the order of the words here to change the emphasis:

  Summary: Audio cuesheet editor

> Source0:        Audio Cuesheet Editor-src.tar.gz

This does not adhere to existing guidelines:
https://fedoraproject.org/wiki/Packaging:SourceURL


> Requires:       gtk-sharp2,mono-core

Are there no automatically added dependencies on what will be needed at
run-time?
https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires


> %description
> Anoyed of

Annoyed


> %setup -q -c %{buildroot}/AudioCuesheetEditor-%{version}

That's not what the %buildroot is for. The source is extracted somewhere in the
$RPM_BUILD_DIR which is neither $RPM_BUILD_ROOT nor %buildroot. Simply use a
relative path here,

  %setup -q -c AudioCuesheetEditor-%{version}

and examine the rpmbuild output to understand how and when this directory is
used and think twice whether you might modify your source release tarball so
that creating this directory will not be necessary anymore. For example, if the
top dir of the source is %{name}-%{version} already, you don't need this -c
option as you can use %setup's default, which is -n %{name}-%{version}.


> ./configure

Add a comment on why %configure is not used and whether any options are not
needed. E.g. whether or why --libdir=%{_libdir} is not needed. See output of
"rpm --eval %configure" for comparison. Getting the configure call right can
become especially important when any paths are built into the software and must
match the location of the files as installed by %make_install (or similar
invocations).


> %install
> rm -rf $RPM_BUILD_ROOT

https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag


> %{_libdir}/%{name}/resources/Texts.xml
> %{_libdir}/%{name}/resources/icons/application-x-cue-16.ico
> ...

https://fedoraproject.org/wiki/Packaging:UnownedDirectories


> %{_libdir}/%{name}/resources/icons/application-x-cue-16.png
> %{_libdir}/%{name}/resources/icons/application-x-cue-22.ico

Maybe you would like to use the '*' wildcard here to reduce the number if
entries? You can also include entire directories with a single line.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=vQT2LpkOS5&a=cc_unsubscribe