Bug 226527 - Merge Review: vino
Summary: Merge Review: vino
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 21:15 UTC by Nobody's working on this, feel free to take it
Modified: 2014-04-02 11:43 UTC (History)
4 users (show)

Fixed In Version: vino-3.12.0-2.fc21
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-04-02 11:43:00 UTC
Type: ---
Embargoed:
panemade: fedora-review+


Attachments (Terms of Use)
spec cleanup as per current packaging guidelines (1.37 KB, patch)
2014-03-26 13:57 UTC, Parag AN(पराग)
no flags Details | Diff
spec cleanup as per current packaging guidelines (1.43 KB, patch)
2014-03-26 14:41 UTC, Parag AN(पराग)
no flags Details | Diff

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

http://cvs.fedora.redhat.com/viewcvs/devel/vino/
Initial Owner: besfahbo

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> - 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.

https://bugzilla.redhat.com/show_bug.cgi?id=959872


> +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.


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