Bug 847684 (mate-notification-da) - Review Request: mate-notification-daemon - Notification daemon for MATE Desktop
Summary: Review Request: mate-notification-daemon - Notification daemon for MATE Desktop
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: mate-notification-da
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: MATE-DE-tracker
TreeView+ depends on / blocked
 
Reported: 2012-08-13 09:36 UTC by Dan Mashal
Modified: 2012-12-06 06:59 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-08-25 02:58:36 UTC
Type: Bug
Embargoed:
rdieter: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Comment 1 Rex Dieter 2012-08-19 00:39:30 UTC
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-19 02:11:59 UTC
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-19 02:28:27 UTC
Fixed that. Thanks Rex.

Comment 5 Dan Mashal 2012-08-19 02:29:14 UTC
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 07:30:48 UTC
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 07:31:43 UTC
I used this provides as per Rex's suggestion.

Comment 8 Wolfgang Ulbrich 2012-08-19 07:58:37 UTC
(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 08:18:43 UTC
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 08:21:45 UTC
(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 12:45:57 UTC
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 Gwyn Ciesla 2012-08-19 19:59:14 UTC
Git done (by process-git-requests).

Fixed branch name.

Comment 13 Fedora Update System 2012-08-24 11:23:31 UTC
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 11:23:42 UTC
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 21:34:50 UTC
mate-notification-daemon-1.4.0-2.fc18 has been pushed to the Fedora 18 testing repository.

Comment 16 Fedora Update System 2012-08-25 02:58:36 UTC
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 06:06:52 UTC
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 06:07:02 UTC
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 06:07:12 UTC
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 04:35:51 UTC
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 04:36:16 UTC
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 22:25:58 UTC
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 23:32:54 UTC
mate-notification-daemon-1.4.0-3.fc18 has been pushed to the Fedora 18 stable repository.

Comment 24 Fedora Update System 2012-09-25 02:48:21 UTC
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-25 02:48:32 UTC
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 19:41:35 UTC
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 19:59:19 UTC
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-26 02:50:43 UTC
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 06:59:21 UTC
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.