Bug 207532

(I'm no official reviewer so the comment is just a proposal)

- Use "%configure --disable-rpath" instead of "./configure --disable-rpath"
- Not sure about "%{__rm} -rf $RPM_BUILD_ROOT". PackagingGuidelines and
ReviewGuidelines are using simple "rm -rf %{buildroot}"
- Not sure about french description. In ReviewGuidelines it is a must, that the
spec is in American English.

(In reply to comment #1)
> (I'm no official reviewer so the comment is just a proposal)
> 
> - Use "%configure --disable-rpath" instead of "./configure --disable-rpath"

Oups... Fixed

> - Not sure about "%{__rm} -rf $RPM_BUILD_ROOT". PackagingGuidelines and
> ReviewGuidelines are using simple "rm -rf %{buildroot}"

When a macro is available for a command, I prefer use it.

> - Not sure about french description. In ReviewGuidelines it is a must, that 
the
> spec is in American English.

I don't remember where I saw it, but other language description and summary 
was a SHOULD

Spec URL: 
http://linuxelectronique.free.fr/download/fedora/5/SPECS/kbackup.spec
SRPM URL: 
http://linuxelectronique.free.fr/download/fedora/5/SRPMS/kbackup-0.4.2-3.src.rpm

%changelog
* Thu Sep 21 2006 Alain Portal <aportal[AT]univ-montp2[DOT]fr> 0.4.2-3
  - Use macro for configure instead of hardcoding path
  - Use macro style instead of variable style

> I don't remember where I saw it, but other 
> language description and summary was a SHOULD

My fault, you're right. Please ignore this.

Just a quick check:
A: Please see http://fedoraproject.org/wiki/Packaging/ScriptletSnippets.
    then:
* Icons are installed under /usr/share/icons/hicolor/. This requires some
  scriptlets.
* Requires(post,postun) should be removed.

B. rpmlint is not silent. I think this can be ignored, however, check if 
you can remove dangling (resolved by kdelibs) symlink.

W: kbackup dangling-symlink /usr/share/doc/HTML/de/kbackup/common
/usr/share/doc/HTML/de/common
W: kbackup symlink-should-be-relative /usr/share/doc/HTML/de/kbackup/common
/usr/share/doc/HTML/de/common
W: kbackup dangling-symlink /usr/share/doc/HTML/en/kbackup/common
/usr/share/doc/HTML/en/common
W: kbackup symlink-should-be-relative /usr/share/doc/HTML/en/kbackup/common
/usr/share/doc/HTML/en/common

Spec URL: 
http://linuxelectronique.free.fr/download/fedora/5/SPECS/kbackup.spec
SRPM URL: 
http://linuxelectronique.free.fr/download/fedora/5/SRPMS/kbackup-0.5-1.src.rpm

%changelog
* Mon Sep 25 2006 Alain Portal <aportal[AT]univ-montp2[DOT]fr> 0.5-1
  - New upstream version
  - Update patch0 and patch1
  - Remove patch2 that is no more needed

* Mon Sep 25 2006 Alain Portal <aportal[AT]univ-montp2[DOT]fr> 0.4.2-4
  - Requires(post,postun) desktop-file-utils no more needed since FC-5
  - Add %%post an %%postun for icons
  - Remove absolute symlinks

I will review this later.

First review of this package.

1. From http://fedoraproject.org/wiki/Packaging/Guidelines :
* Use rpmlint
  - rpmlint is not silent.
//usr/share/applications/kde/kbackup.desktop: error: required key "Encoding" not
found
E: kbackup invalid-desktopfile //usr/share/applications/kde/kbackup.desktop
   This is perhaps because kbackup.desktop doesn't have the entry of
   Encoding=UTF-8.

* Desktop files
  - Well, in GNOME I can see two desktop entries of KBackup under
  "system tools".  Perhaps either desktop file should have
  "OnlyShowIn=KDE;" entry and the other "OnlyShowIn=GNOME;"

* File and Directory Ownership
  The following directries are owned by other packages and
  should not be owned by this package.

/usr/share/applications/kde
/usr/share/icons/hicolor
/usr/share/icons/hicolor/16x16
/usr/share/icons/hicolor/16x16/apps
/usr/share/icons/hicolor/16x16/mimetypes
/usr/share/icons/hicolor/32x32
/usr/share/icons/hicolor/32x32/apps
/usr/share/icons/hicolor/32x32/mimetypes

2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
   = Nothing.

3. Other things I have noticed:
* I recommend that %configure is moved to %build stage.

(In reply to comment #7)

> * Desktop files
>   - Well, in GNOME I can see two desktop entries of KBackup under
>   "system tools".  Perhaps either desktop file should have
>   "OnlyShowIn=KDE;" entry and the other "OnlyShowIn=GNOME;"

If I add "OnlyShowIn=KDE;", that means the desktop entry won't be in Gnome 
menu? (I'm using kde and I have only one entry in Utilities->File)
Perhaps should I have to remove "System;" from Categories?

Spec URL: 
http://linuxelectronique.free.fr/download/fedora/5/SPECS/kbackup.spec
SRPM URL: 
http://linuxelectronique.free.fr/download/fedora/5/SRPMS/kbackup-0.5-2.src.rpm

%changelog
* Mon Sep 25 2006 Alain Portal <aportal[AT]univ-montp2[DOT]fr> 0.5-2
  - Use macro for make
  - Don't own some directories
  - Update patch0 and patch1
  - Improve desktop-file installation

* Well, currently in GNOME only one menu appears for kbackup,
however, my opinion is "OnlyShowIn=KDE;" should be in
/usr/share/applications/kde/kbackup.desktop, not in
/usr/share/applications/fedora-kbackup.desktop.

I can see that /usr/share/applications/kde/kresources.desktop
(in kdelibs) has the entry "OnlyShowIn=KDE;".


* Another thing: /usr/share/applications/kde/kbackup.desktop has
X-SuSE-Backup in Category, this should be changed.

Spec URL: 
http://linuxelectronique.free.fr/download/fedora/5/SPECS/kbackup.spec
SRPM URL: 
http://linuxelectronique.free.fr/download/fedora/5/SRPMS/kbackup-0.5-3.src.rpm

%changelog
* Mon Sep 25 2006 Alain Portal <aportal[AT]univ-montp2[DOT]fr> 0.5-3
  - desktop-file-install don't work as I expected, so update patch0

I am pleased to say that this package is now APPROVED.

FYI, you shouldn't have 2 .desktop files, I'd recommend changing
mkdir -p %{buildroot}%{_datadir}/applications
desktop-file-install --vendor fedora \
  --dir %{buildroot}%{_datadir}/applications \
  --add-category "Utility" \
  --add-category "X-KDE-Utilities-File" \
  --add-category "X-Fedora" \
  --remove-only-show-in "KDE" \
  src/%{name}.desktop

And,
# Fix absolute symlink
%{__rm} -f %{buildroot}%{_docdir}/HTML/*/%{name}/common

you remove the symlink, but then don't ever actually replace/fix it.  Either 
fix it to be a relative symlink or leave as is.  Else, you'll end up with 
broken help links.

Crap, my comment #13 got truncated, it was *supposed* to say change to

desktop-file-install --vendor="" \
  --dir %{buildroot}%{_datadir}/applications/kde \
  --add-category="Utility" \
  --add-category="X-KDE-Utilities-File" \
  --add-category="X-Fedora" \
  --remove-only-show-in "KDE" \
  %{buildroot}%{_datadir}/applications/kde/*.desktop

(In reply to comment #14)
> And,
> # Fix absolute symlink
> %{__rm} -f %{buildroot}%{_docdir}/HTML/*/%{name}/common
> 
> you remove the symlink, but then don't ever actually replace/fix it.  Either 
> fix it to be a relative symlink or leave as is.  Else, you'll end up with 
> broken help links.

Please, could you tell me how to fix?

(In reply to comment #16)
> (In reply to comment #14)
> > And,
> > # Fix absolute symlink
> > %{__rm} -f %{buildroot}%{_docdir}/HTML/*/%{name}/common
> > 
> > you remove the symlink, but then don't ever actually replace/fix it.  Either 
> > fix it to be a relative symlink or leave as is.  Else, you'll end up with 
> > broken help links.
> 
> Please, could you tell me how to fix?
> 

Simply not removing %{buildroot}%{_docdir}/HTML/*/%{name}/common
is the easiest idea to follow the recommendation from Rex.
I don't complain for it.

I found two desktop entries in my menu
- one in "System"
- one in "Utilities->File"

It seems to me that the better place is "Utilities->File".
To remove the one in "System", I think that I have to remove "System" from the 
categories field in desktop. But if I do that, it seems to me there won't be 
any entry in a GNOME menu.
Is there a GNOME category similar to "X-KDE-Utilities-File"?

Well, if it is complicated to unify desktop files, I think
owing two desktop files is not so bad idea.

Spec URL: 
http://linuxelectronique.free.fr/download/fedora/5/SPECS/kbackup.spec
SRPM URL: 
http://linuxelectronique.free.fr/download/fedora/5/SRPMS/kbackup-0.5-4.src.rpm

%changelog
* Mon Sep 25 2006 Alain Portal <aportal[AT]univ-montp2[DOT]fr> 0.5-4
  - Install only one desktop file
  - Don't remove absolute symlinks
  - Update patch0

(In reply to comment #20)
> Spec URL: 
> http://linuxelectronique.free.fr/download/fedora/5/SPECS/kbackup.spec
> SRPM URL: 
> http://linuxelectronique.free.fr/download/fedora/5/SRPMS/kbackup-0.5-4.src.rpm
> 
> %changelog
> * Mon Sep 25 2006 Alain Portal <aportal[AT]univ-montp2[DOT]fr> 0.5-4
>   - Install only one desktop file
>   - Don't remove absolute symlinks
>   - Update patch0

This time, in GNOME desktop entry appears on "accessory".
For KDE, where does the entry for KBackup appear?

Well, if desktop entry appears in "Utilities->File" on KDE as
you have expected, please go ahead (i.e. import to cvs).

I don't stop you from commiting this package to cvs.

Well, I'll commit as is, but I'll wish to improve for the GNOME menu, so, if 
you know a better place in the menu than "Accessory", let me know.

%changelog
* Wed Sep 26 2006 Alain Portal <aportal[AT]univ-montp2[DOT]fr> 0.5-6
  - Link the good directories

* Tue Sep 26 2006 Alain Portal <aportal[AT]univ-montp2[DOT]fr> 0.5-5
  - Fix absolute symlinks

Imported and built.

Package Change Request
======================
Package Name: kbackup
Updated Fedora Owners: alain.portal

Please, add my home email in comps because I'm on vacation for 6 weeks.

added

Package Change Request
======================
Package Name: kbackup
New Branches: F-11 F-12
Owners: dionysos

kbackup is already orphaned:
http://www.redhat.com/archives/fedora-devel-list/2009-March/msg00093.html
http://www.redhat.com/archives/fedora-devel-list/2009-March/msg00103.html

and:
http://koji.fedoraproject.org/koji/packageinfo?packageID=2191
(see tags and included? column)

It is already more than 6 months and to reintroduce this package
into Fedora again, the new review request is needed. Please
submit the new one, thank you.

(In reply to comment #28)
> It is already more than 6 months and 
should be read as:
It is already more than 6 months since this package was orphaned and

OK, I´ll submit a new review request.
Thanks.