https://bugzilla.redhat.com/show_bug.cgi?id=978489 Mathieu Bridon <bochecha@xxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review+ --- Comment #3 from Mathieu Bridon <bochecha@xxxxxxxxxxxxxxxxx> --- (In reply to Debarshi Ray from comment #2) > Spec: http://rishi.fedorapeople.org/dleyna-connector-dbus.spec > SRPM: > http://rishi.fedorapeople.org/dleyna-connector-dbus-0.1.0-2.fc19.src.rpm > > (In reply to Mathieu Bridon from comment #1) > > Package is almost good to go, there's just a few little things to fix. > > > > Summary > > ======= > > > > [!]: Package requires other packages for directories it uses. > > [!]: Package must own all directories that it creates. > > > > => Package installs > > %{_libdir}/dleyna-%{api}/connectors/libdleyna-connector-dbus.so > > but does not provide the %{_libdir}/dleyna-%{api} and > > %{_libdir}/dleyna-%{api}/connectors folders. > > > > => Maybe the right fix here would be to have dleyna-core provide these > > folders (it does not at the moment) and then dleyna-connector-dbus > > could just explicitly require dleyna-core? > > Good catch. To me it seemed more sane to have dleyna-connector-dbus own > those two directories. eg., I don't know if other dLeyna components will be > creating more such directories. Up to you. I would have seen it owned by dleyna-core because the name made it sound like there might be other connectors which would drop stuff in there. But this works for me, as long as the dir is owned by something. > > [!]: Final provides and requires are sane (see attachments). > > > > => Should the private .so file be filtered from provides? > > This is a real question, I can't answer it as I don't know what this > > file is supposed to do. I'll trust your judgement on this point. > > I don't know either. dLeyna supports different IPC mechanisms (see > connector-name in /etc/dleyna-renderer-service.conf) via a plugin > architecture. This particular file implements support for D-Bus. What will load this plugin? If it's something which just loads whatever is installed and configured, then there wouldn't be any requirement, and as such I'd filter the Provides out. If it's something that needs to link against it, then the Provides should stay. But these are just examples I can think of off the top of my head, this package could be in a different situation, dunno. In any case, the Provides shouldn't cause any harm (e.g given its name, it's unlikely to conflict with anything else), so I won't block the review on this. ---------- All my issues with the package are fixed by this new one, see diff below. $ diff -u dleyna-connector-dbus.spec.old dleyna-connector-dbus.spec --- dleyna-connector-dbus.spec.old 2013-06-27 01:32:08.000000000 +0800 +++ dleyna-connector-dbus.spec 2013-07-08 19:25:53.000000000 +0800 @@ -1,8 +1,8 @@ -%define api 1.0 +%global api 1.0 Name: dleyna-connector-dbus Version: 0.1.0 -Release: 1%{?dist} +Release: 2%{?dist} Summary: D-Bus connector for dLeyna services License: LGPLv2 @@ -48,6 +48,8 @@ %doc COPYING %doc ChangeLog %doc README +%dir %{_libdir}/dleyna-%{api} +%dir %{_libdir}/dleyna-%{api}/connectors %{_libdir}/dleyna-%{api}/connectors/libdleyna-connector-dbus.so %files devel @@ -55,5 +57,9 @@ %changelog +* Sun Jul 07 2013 Debarshi Ray <rishi@xxxxxxxxxxxxxxxxx> - 0.1.0-2 +- Use %%global instead of %%define. +- Own %%{_libdir}/dleyna-%%{api} and %%{_libdir}/dleyna-%%{api}/connectors. + * Wed Jun 26 2013 Debarshi Ray <rishi@xxxxxxxxxxxxxxxxx> - 0.1.0-1 - Initial spec. ---------- Package is approved. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=jRU9p7YJgr&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review