Bug 847684 - (mate-notification-da) Review Request: mate-notification-daemon - Notification daemon for MATE Desktop
Review Request: mate-notification-daemon - Notification daemon for MATE Desktop
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Rex Dieter
Fedora Extras Quality Assurance
:
Depends On:
Blocks: MATE-DE-tracker
  Show dependency treegraph
 
Reported: 2012-08-13 05:36 EDT by Dan Mashal
Modified: 2012-12-06 01:59 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-08-24 22:58:36 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rdieter: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Comment 1 Rex Dieter 2012-08-18 20:39:30 EDT
naming: ok

1.  SHOULD move
NOCONFIGURE=1 ./autogen.sh
to  %setup section

2. MUST not use undefined macro %po_package, either  define it or use %{name} instead

3. SHOULD add
Provides: desktop-notification-daemon
(assuming it provides dbus service org.freedesktop.Notifications like kde's knotify or gnome's notification-daemon)

4. SHOULD consider dropping -libs subpkg, I don't see any shared libraries in use here, so
%files libs
%{_libdir}/mate-notification-daemon/
could probably just be folded into the main package.

licensing: ok

sources: ok
md5sum *.xz
83956e38caeec78634af379abc1b5787  mate-notification-daemon-1.4.0.tar.xz

scriptlets: ok
Comment 3 Rex Dieter 2012-08-18 22:11:59 EDT
Noticed some small  errors in scriptlets now too (sorry):

%pre
%gconf_schema_prepare %{buildroot}/%{_sysconfdir}/mateconf/schemas/mate-notification-daemon.schemas

%post -p /sbin/ldconfig
/usr/bin/update-desktop-database &>/dev/null || :
/bin/touch --no-create %{_datadir}/icons/mate &>/dev/null || :
%gconf_schema_upgrade %{buildroot}/%{_sysconfdir}/mateconf/schemas/mate-notification-daemon.schemas

%postun -p /sbin/ldconfig
/usr/bin/update-desktop-database &>/dev/null || :
%gconf_schema_remove %{buildroot}/%{_sysconfdir}/mateconf/schemas/mate-notification-daemon.schemas
if [ $1 -eq 0 ] ; then
    /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null
    /usr/bin/gtk-update-icon-cache -f %{_datadir}/icons/hicolor &>/dev/null || :
fi

%posttrans
/usr/bin/gtk-update-icon-cache -f %{_datadir}/icons/hicolor &>/dev/null || :

should be (something like):

%pre
%gconf_schema_prepare %{_sysconfdir}/mateconf/schemas/mate-notification-daemon.schemas

%post
/usr/bin/update-desktop-database &>/dev/null || :
/bin/touch --no-create %{_datadir}/icons/mate &>/dev/null || :
%gconf_schema_upgrade %{_sysconfdir}/mateconf/schemas/mate-notification-daemon.schemas

%postun
/usr/bin/update-desktop-database &>/dev/null || :
%gconf_schema_remove %{_sysconfdir}/mateconf/schemas/mate-notification-daemon.schemas
if [ $1 -eq 0 ] ; then
    /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null
    /usr/bin/gtk-update-icon-cache -f %{_datadir}/icons/hicolor &>/dev/null || :
fi

%posttrans
/usr/bin/gtk-update-icon-cache -f %{_datadir}/icons/hicolor &>/dev/null || :


please fix that prior to submitting any builds.



APPROVED.
Comment 4 Dan Mashal 2012-08-18 22:28:27 EDT
Fixed that. Thanks Rex.
Comment 5 Dan Mashal 2012-08-18 22:29:14 EDT
New Package SCM Request
=======================
Package Name: mate-notification-daemon
Short Description: Notification daemon for MATE Desktop
Owners: vicodan rdieter raveit65
Branches: f16 f17 18
InitialCC:
Comment 6 Wolfgang Ulbrich 2012-08-19 03:30:48 EDT
You use wrong provides.
Provides:	desktop-notification-daemon

A provide with the same name as the package itself makes no sense.
Pls, use instead this provides before you import. This is important for a working desktop.

Provides: mate-notification-daemon-engine-slider%{?_isa} = %{version}-%{release}
Provides: mate-notification-daemon-engine-nodoka%{?_isa} = %{version}-%{release}
Provides: mate-notification-daemon-engine-coco%{?_isa} = %{version}-%{release}
Comment 7 Dan Mashal 2012-08-19 03:31:43 EDT
I used this provides as per Rex's suggestion.
Comment 8 Wolfgang Ulbrich 2012-08-19 03:58:37 EDT
(In reply to comment #7)
> I used this provides as per Rex's suggestion.
Than use
Provides:	desktop-notification-daemon
Provides: mate-notification-daemon-engine-slider%{?_isa} = %{version}-%{release}
Provides: mate-notification-daemon-engine-nodoka%{?_isa} = %{version}-%{release}
Provides: mate-notification-daemon-engine-coco%{?_isa} = %{version}-%{release}

Doing someting double is in this case not wrong. ;)

This you can also fix.

desktop-file-install									\
	--remove-category="MATE"	\
	--add-category="X-Mate"							\
	--remove-only-show-in="MATE"						\
	--add-only-show-in="X-MATE"						\
	--delete-original							\
	--dir=%{buildroot}%{_datadir}/applications					\
%{buildroot}/%{_datadir}/applications/mate-notification-properties.desktop

Here you can remove 
--add-category="X-Mate"
because we don't need category desktop anymore. see
https://bugs.freedesktop.org/show_bug.cgi?id=52493
I talked with Steve Zech from upstream about this, he agree with that.

Note: this package can't update a package from my repo with old scriplet style.
I tested this aboout 40 hours the last week, with help from Leigh Scott.
{_sysconfdir}/mateconf/schemas/mate-notification-daemon.schemas wouldn't be writen correct or not at all in user mateconf directory.
It is impossible. Sorry
So no need to to push this to bodhi for fc17/16.

This applies for all packages which use a mateconf scheme in {_sysconfdir}/mateconf/schemas/*.
Comment 9 Wolfgang Ulbrich 2012-08-19 04:18:43 EDT
see https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#GConf

Requires(pre): mate-conf
Requires(preun): mate-conf
Requires(post): mate-conf
Requires:	mate-desktop
Comment 10 Wolfgang Ulbrich 2012-08-19 04:21:45 EDT
(In reply to comment #9)
> see https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#GConf
> 
> Requires(pre): mate-conf
> Requires(preun): mate-conf
> Requires(post): mate-conf
> Requires:	mate-desktop

sorry, better

Requires(pre): mate-conf
Requires(post): mate-conf
Requires(preun): mate-conf
Comment 11 Rex Dieter 2012-08-19 08:45:57 EDT
For the scriptlets, they can just  be fixed after importing.

I don't follow why you think these are needed:
Provides: mate-notification-daemon-engine-slider%{?_isa} = %{version}-%{release}
Provides: mate-notification-daemon-engine-nodoka%{?_isa} = %{version}-%{release}
Provides: mate-notification-daemon-engine-coco%{?_isa} = %{version}-%{release}

does your packaging of mate use and need these somehow?
Comment 12 Jon Ciesla 2012-08-19 15:59:14 EDT
Git done (by process-git-requests).

Fixed branch name.
Comment 13 Fedora Update System 2012-08-24 07:23:31 EDT
mate-notification-daemon-1.4.0-2.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-notification-daemon-1.4.0-2.fc17
Comment 14 Fedora Update System 2012-08-24 07:23:42 EDT
mate-notification-daemon-1.4.0-2.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/mate-notification-daemon-1.4.0-2.fc18
Comment 15 Fedora Update System 2012-08-24 17:34:50 EDT
mate-notification-daemon-1.4.0-2.fc18 has been pushed to the Fedora 18 testing repository.
Comment 16 Fedora Update System 2012-08-24 22:58:36 EDT
mate-notification-daemon-1.4.0-2.fc17 has been pushed to the Fedora 17 stable repository.
Comment 17 Fedora Update System 2012-08-27 02:06:52 EDT
mate-notification-daemon-1.4.0-3.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-notification-daemon-1.4.0-3.fc17
Comment 18 Fedora Update System 2012-08-27 02:07:02 EDT
mate-notification-daemon-1.4.0-3.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/mate-notification-daemon-1.4.0-3.fc16
Comment 19 Fedora Update System 2012-08-27 02:07:12 EDT
mate-notification-daemon-1.4.0-3.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/mate-notification-daemon-1.4.0-3.fc18
Comment 20 Fedora Update System 2012-09-16 00:35:51 EDT
mate-notification-daemon-1.4.0-6.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-notification-daemon-1.4.0-6.fc17
Comment 21 Fedora Update System 2012-09-16 00:36:16 EDT
mate-notification-daemon-1.4.0-6.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/mate-notification-daemon-1.4.0-6.fc18
Comment 22 Fedora Update System 2012-09-17 18:25:58 EDT
mate-notification-daemon-1.4.0-2.fc18 has been pushed to the Fedora 18 stable repository.
Comment 23 Fedora Update System 2012-09-17 19:32:54 EDT
mate-notification-daemon-1.4.0-3.fc18 has been pushed to the Fedora 18 stable repository.
Comment 24 Fedora Update System 2012-09-24 22:48:21 EDT
mate-notification-daemon-1.4.0-8.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/mate-notification-daemon-1.4.0-8.fc18
Comment 25 Fedora Update System 2012-09-24 22:48:32 EDT
mate-notification-daemon-1.4.0-8.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-notification-daemon-1.4.0-8.fc17
Comment 26 Fedora Update System 2012-09-30 15:41:35 EDT
mate-notification-daemon-1.4.0-9.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-notification-daemon-1.4.0-9.fc17
Comment 27 Fedora Update System 2012-09-30 15:59:19 EDT
mate-notification-daemon-1.4.0-9.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/mate-notification-daemon-1.4.0-9.fc18
Comment 28 Fedora Update System 2012-11-25 21:50:43 EST
mate-notification-daemon-1.5.0-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/mate-notification-daemon-1.5.0-1.fc16
Comment 29 Fedora Update System 2012-12-06 01:59:21 EST
mate-notification-daemon-1.5.0-1.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.

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