Bug 226527 - Merge Review: vino
Merge Review: vino
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 16:15 EST by Nobody's working on this, feel free to take it
Modified: 2014-04-02 07:43 EDT (History)
4 users (show)

See Also:
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 07:43:00 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
panemade: fedora‑review+


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

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 16:15:14 EST
Fedora Merge Review: vino

http://cvs.fedora.redhat.com/viewcvs/devel/vino/
Initial Owner: besfahbo@redhat.com
Comment 1 Parag AN(पराग) 2014-03-26 09:56:46 EDT
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 09:57:49 EDT
Created attachment 879007 [details]
spec cleanup as per current packaging guidelines
Comment 3 Kalev Lember 2014-03-26 10:16:23 EDT
> -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 10:20:25 EDT
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 10:33:54 EDT
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 10:41:45 EDT
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 07:43:00 EDT
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.