[Bug 226036] Merge Review: liboil
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226036 Orcan 'oget' Ogetbil changed: What|Removed |Added Status|ASSIGNED|CLOSED Resolution||RAWHIDE Flag|fedora-review? |fedora-review+ --- Comment #7 from Orcan 'oget' Ogetbil 2009-06-10 13:35:55 EDT --- All issues have been addressed. Thanks for the updates. -- This merge review (liboil) is APPROVED by oget -- Closing. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226036] Merge Review: liboil
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226036 --- Comment #6 from Bastien Nocera 2009-06-10 13:20:42 EDT --- Done in -3 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226036] Merge Review: liboil
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226036 --- Comment #5 from Orcan 'oget' Ogetbil 2009-06-10 13:08:35 EDT --- Thank you Bastien, There is this minor issue that is unfortunately a blocker: The "%doc HACKING" line must come after "defattr(-,root,root,-)" Please fix this so we can close the bug. Thanks again! -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226036] Merge Review: liboil
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226036
--- Comment #4 from Bastien Nocera 2009-06-10 10:46:09
EDT ---
(In reply to comment #1)
> I reviewed this package. There are some issues, questions and suggestions that
> I'll bring into your attention.
>
> * The latest version is not packaged! Please update to the latest version.
Latest version is in devel
> * The HACKING file should be packaged, possibly in the %doc of devel
Done
> ? The examples directory can go into the %doc of devel too.
The code in examples are too complicated to be used stand-alone.
> ? Some files in m4 and the file "missing" indicates GPLv2+ license.But I
> don't
> think these make their way into the package, so I guess BSD is good enough.
OK
> * I don't think glib2-devel is required to build the package. I built an
> identical package in mock without BR'in glib2-devel. Some of the examples
> require glib2-devel. So if you are going to package those examples you'll need
> to require glib2-devel in the devel subpackage.
It's optional, but used. From configure.ac:
PKG_CHECK_MODULES(GLIB, glib-2.0, HAVE_GLIB=yes, HAVE_GLIB=no)
AC_SUBST(GLIB_LIBS)
AC_SUBST(GLIB_CFLAGS)
AC_ARG_ENABLE(glib,
AC_HELP_STRING([--disable-glib],[disable usage of glib]),
[case "${enableval}" in
yes) HAVE_GLIB=yes ;;
no) HAVE_GLIB=no ;;
*) AC_MSG_ERROR(bad value ${enableval} for --disable-glib) ;;
esac])
AM_CONDITIONAL(HAVE_GLIB, test "x$HAVE_GLIB" = "xyes")
> * A package must own all directories that it creates. If it does not create a
> directory that it uses, then it should require a package which does create
> that
> directory. The directory %{_datadir}/gtk-doc/html/ is created but not owned.
> So
> the devel package should require "gtk-doc" which is the rightful owner of that
> directory.
Done
> ! Try to make use of the %{name} macro.
Done in the few places where it makes sense.
> * From the SPEC file:
># multi-jobbed make makes the build fail:
># ./build_prototypes_doc >liboilfuncs-doc.h
># /bin/sh: ./build_prototypes_doc: No such file or directory
>make %{?_smp_mflags}
> Isn't there a contradiction here? What is that commented-out section for?
I removed the comment.
> * From the SPEC file:
># Disable Altivec, so that liboil doesn't SIGILL on non-Altivec PPCs
># See https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=252179#c15
>#sed -i 's/CFLAGS="$CFLAGS "-maltivec""/CFLAGS="$CFLAGS
> "-fno-tree-vectorize
> -Wa,-maltivec""/' configure
>#sed -i 's/LIBOIL_CFLAGS -maltivec/LIBOIL_CFLAGS -fno-tree-vectorize
> -Wa,-maltivec/' configure
> Do we still need these lines in the SPEC file?
Nope, removed.
Should all be fixed in 0.3.16-2
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226036] Merge Review: liboil
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226036 Orcan 'oget' Ogetbil changed: What|Removed |Added CC||bnoc...@redhat.com --- Comment #3 from Orcan 'oget' Ogetbil 2009-06-09 15:05:03 EDT --- re-ping? I added more previous maintainers to the CC. Please DO NOT IGNORE! -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226036] Merge Review: liboil
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226036 --- Comment #2 from Orcan 'oget' Ogetbil 2009-04-04 01:44:09 EDT --- ping? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226036] Merge Review: liboil
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226036
Orcan 'oget' Ogetbil changed:
What|Removed |Added
Status|NEW |ASSIGNED
CC||caol...@redhat.com,
||oget.fed...@gmail.com,
||walt...@redhat.com
AssignedTo|nob...@fedoraproject.org|oget.fed...@gmail.com
Flag||fedora-review?
--- Comment #1 from Orcan 'oget' Ogetbil 2009-01-17
13:27:02 EDT ---
I reviewed this package. There are some issues, questions and suggestions that
I'll bring into your attention.
* The latest version is not packaged! Please update to the latest version.
* The HACKING file should be packaged, possibly in the %doc of devel
? The examples directory can go into the %doc of devel too.
? Some files in m4 and the file "missing" indicates GPLv2+ license.But I don't
think these make their way into the package, so I guess BSD is good enough.
* I don't think glib2-devel is required to build the package. I built an
identical package in mock without BR'in glib2-devel. Some of the examples
require glib2-devel. So if you are going to package those examples you'll need
to require glib2-devel in the devel subpackage.
* A package must own all directories that it creates. If it does not create a
directory that it uses, then it should require a package which does create that
directory. The directory %{_datadir}/gtk-doc/html/ is created but not owned. So
the devel package should require "gtk-doc" which is the rightful owner of that
directory.
! Try to make use of the %{name} macro.
* From the SPEC file:
# multi-jobbed make makes the build fail:
# ./build_prototypes_doc >liboilfuncs-doc.h
# /bin/sh: ./build_prototypes_doc: No such file or directory
make %{?_smp_mflags}
Isn't there a contradiction here? What is that commented-out section for?
* From the SPEC file:
# Disable Altivec, so that liboil doesn't SIGILL on non-Altivec PPCs
# See https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=252179#c15
#sed -i 's/CFLAGS="$CFLAGS "-maltivec""/CFLAGS="$CFLAGS "-fno-tree-vectorize
-Wa,-maltivec""/' configure
#sed -i 's/LIBOIL_CFLAGS -maltivec/LIBOIL_CFLAGS -fno-tree-vectorize
-Wa,-maltivec/' configure
Do we still need these lines in the SPEC file?
Adding Colin and Caolan to CC since they were the last two known maintainers.
Sorry if this was not desired
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
