Bug 997990 - Review Request: xfce4-vala - Vala bindings for the Xfce framework
Review Request: xfce4-vala - Vala bindings for the Xfce framework
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kalev Lember
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2013-08-16 13:31 EDT by Kevin Fenzi
Modified: 2015-05-25 13:32 EDT (History)
2 users (show)

See Also:
Fixed In Version: xfce4-vala-4.10.3-1.fc19
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2013-08-31 19:56:00 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kalevlember: fedora‑review+
limburgher: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Kevin Fenzi 2013-08-16 13:31:46 EDT
Spec URL: http://www.scrye.com/~kevin/fedora/review/xfce4-vala/xfce4-vala.spec
SRPM URL: http://www.scrye.com/~kevin/fedora/review/xfce4-vala/xfce4-vala-4.10.3-0.1.git20130815.fc20.src.rpm
Xfce4 Vala provides bindings for the Xfce framework

Fedora Account System Username: kevin

Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5822992
xfce4-vala.noarch: W: devel-file-in-non-devel-package /usr/share/pkgconfig/xfce4-vala.pc
(can be ignored I think).

Some notes: I am using a git snapshot because the latest release requires vala 0.16, and we have 0.20. There's a commit in git that adds ability to specify vala version that we need. 

There's also a patch to make this package noarch and a upstream bug on the same. I don't see why it needs to be archfull, but if my reviewer does, feel free to convince me. :)
Comment 1 Kalev Lember 2013-08-19 15:26:25 EDT
Taking for review.

Can you update to the new 4.10.3 release that has the patches?

> BuildRequires: vala-devel
> BuildRequires: libxfce4util-devel
> BuildRequires: garcon-devel
> BuildRequires: xfconf-devel
> BuildRequires: libxfce4ui-devel
> BuildRequires: xfce4-panel-devel
> BuildRequires: exo-devel
> BuildRequires: xfce4-dev-tools

Minor nitpick: these would look nicer sorted.

> Requires: pkgconfig

rpmbuild autogenerates the pkgconfig dep for all packages that ship a .pc file; no need to do it manually.

> %configure --with-vala-api=0.20

I'm building vala 0.21 for rawhide right now, can you update this to pass API version 0.22 ?

> %{_datadir}/vala-0.20/vapi/*

Should depend on the vala package for the /usr/share/vala-*/ directory ownership.
Comment 2 Kevin Fenzi 2013-08-19 20:49:01 EDT
Updated package, I think I addressed all your comments, please doublecheck: 

Spec URL: http://www.scrye.com/~kevin/fedora/review/xfce4-vala/xfce4-vala.spec
SRPM URL: http://www.scrye.com/~kevin/fedora/review/xfce4-vala/xfce4-vala-4.10.3-1.fc20.src.rpm
Comment 3 Kalev Lember 2013-08-20 16:56:48 EDT
Fedora review xfce4-vala-4.10.3-1.fc20.src.rpm 2013-08-20

$ rpmlint xfce4-vala-4.10.3-1.fc20.src.rpm \
xfce4-vala.src:19: W: macro-in-comment %{_datadir}
xfce4-vala.noarch: W: devel-file-in-non-devel-package /usr/share/pkgconfig/xfce4-vala.pc
2 packages and 0 specfiles checked; 0 errors, 2 warnings.

+ OK
! needs attention

+ rpmlint errors are harmless and can be ignored
+ The package is named according to Fedora packaging guidelines
+ The spec file name matches the base package name.
+ The package meets the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the
  Licensing Guidelines.
+ The license field in the spec file matches the actual license
+ The package contains the license file (COPYING)
+ Spec file is written in American English
+ Spec file is legible
+ Upstream sources match sources in the srpm. md5sum:
  0bbb1d6e473e0fe9b335b7b1b49d8a71  xfce4-vala-4.10.3.tar.bz2
  0bbb1d6e473e0fe9b335b7b1b49d8a71  Download/xfce4-vala-4.10.3.tar.bz2
+ The package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
n/a The spec file handles locales properly
n/a ldconfig in %post and %postun
+ Package does not bundle copies of system libraries
n/a Package isn't relocatable
+ Package owns all the directories it creates
+ No duplicate files in %files
+ Permissions are properly set
+ Consistent use of macros
+ The package must contain code or permissible content
n/a Large documentation files should go in -doc subpackage
+ Files marked %doc should not affect package
n/a Header files should be in -devel
n/a Static libraries should be in -static
n/a Library files that end in .so must go in a -devel package
n/a -devel must require the fully versioned base
+ Packages should not contain libtool .la files
n/a Proper .desktop file handling
+ Doesn't own files or directories already owned by other packages
+ Filenames are valid UTF-8

Looks good to me, just one comment:

> /usr/bin/xdt-autogen --with-vala-api=0.22
> %configure --with-vala-api=0.22

It's probably unnecessary to keep the autogen line now that you're building
from a release tarball, but if you do, I would recommend passing NOCONFIGURE=1
to it so that it doesn't invoke configure:

NOCONFIGURE=1 xdt-autogen 
%configure --with-vala-api=0.22

That way you can also avoid having to duplicate the configure args in two places.

Comment 4 Kevin Fenzi 2013-08-20 18:38:35 EDT
Ah yeah, thats leftovers from building from a snapshot. I'm happy dropping xfce4-dev-tools, xdt-autogen, etc... ;) 

Will fix before importing. Thanks!

New Package SCM Request
Package Name: xfce4-vala
Short Description: Vala bindings for the Xfce framework
Owners: kevin
Branches: f19 f20 devel
Comment 5 Gwyn Ciesla 2013-08-22 08:23:37 EDT
Git done (by process-git-requests).
Comment 6 Fedora Update System 2013-08-22 13:19:43 EDT
xfce4-vala-4.10.3-1.fc19 has been submitted as an update for Fedora 19.
Comment 7 Fedora Update System 2013-08-23 20:04:22 EDT
xfce4-vala-4.10.3-1.fc19 has been pushed to the Fedora 19 testing repository.
Comment 8 Fedora Update System 2013-08-31 19:56:00 EDT
xfce4-vala-4.10.3-1.fc19 has been pushed to the Fedora 19 stable repository.

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