Bug 226527

Summary: Merge Review: vino
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Parag AN(पराग) <panemade>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: behdad, kalevlember, mattdm, panemade
Target Milestone: ---Flags: panemade: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: vino-3.12.0-2.fc21 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-04-02 11:43:00 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Description Flags
spec cleanup as per current packaging guidelines
spec cleanup as per current packaging guidelines none

Description Nobody's working on this, feel free to take it 2007-01-31 21:15:14 UTC
Fedora Merge Review: vino

Initial Owner: besfahbo@redhat.com

Comment 1 Parag AN(पराग) 2014-03-26 13:56:46 UTC
some issues found

- Package does not install a %{name}.desktop using desktop-file-install or desktop-file-validate if there is such a file.

- Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/telepathy/clients,
     /usr/share/dbus-1, /usr/share/telepathy, /usr/share/dbus-1/services

- rpmlint output is
vino.x86_64: W: non-conffile-in-etc /etc/xdg/autostart/vino-server.desktop
vino.x86_64: E: incorrect-fsf-address /usr/share/doc/vino/COPYING
vino.src: E: specfile-error warning: bogus date in %changelog: Sat Oct 22 2006 Matthias Clasen <mclasen@redhat.com> - 2.16.0-1
2 packages and 0 specfiles checked; 2 errors, 1 warnings.

Comment 2 Parag AN(पराग) 2014-03-26 13:57:49 UTC
Created attachment 879007 [details]
spec cleanup as per current packaging guidelines

Comment 3 Kalev Lember 2014-03-26 14:16:23 UTC
> -make install DESTDIR=$RPM_BUILD_ROOT
> +make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"

I'm not much fan of adding overrides like that. Could you perhaps switch to using plain %make_install instead?

There's an rpm ticket open to add the "install -p" override to the  %make_install macro, so that all users of the macro would get this automatically. I would prefer to leave "install -p" out from here and rely on %make_install doing the right thing.


> +desktop-file-validate $RPM_BUILD_ROOT%{_sysconfdir}/xdg/autostart/vino-autostart.desktop || :

There's no value in validating the desktop file if you discard the exit code with "|| :". The reason why the packaging guidelines mandate that all desktop files are validated is to ensure that broken desktop files don't get in the distribution.

Other changes look good to me. And thanks for reviewing these packages!

Comment 4 Parag AN(पराग) 2014-03-26 14:20:25 UTC
Thanks for your quick reply. Hope to see required changes applied in rawhide and once its done this review can be closed.

Comment 5 Kalev Lember 2014-03-26 14:33:54 UTC
If you provide a git formatted patch, I can apply the changes you -- or feel free to push them yourself if you want to, I know you are a provenpackager.

Comment 6 Parag AN(पराग) 2014-03-26 14:41:45 UTC
Created attachment 879024 [details]
spec cleanup as per current packaging guidelines

I am okay with any way to fix this packaging issues. I will commit this patch tomorrow.

Comment 7 Parag AN(पराग) 2014-04-02 11:43:00 UTC
I have committed this last week and fixed in vino-3.12.0-2.fc21

APPROVED this build and package.