Bug 226551

Summary: Merge Review: xchat
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Christopher Aillon <caillon>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: bdpepple, caillon, fedora, kevin
Target Milestone: ---Flags: bdpepple: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 2.8.2-12.fc8 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-07-03 20:30:22 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Nobody's working on this, feel free to take it 2007-01-31 21:18:19 UTC
Fedora Merge Review: xchat

http://cvs.fedora.redhat.com/viewcvs/devel/xchat/
Initial Owner: caillon

Comment 1 Brian Pepple 2007-02-03 21:20:30 UTC
Good:
* Source URL is canonical
* Group Tag is from the official list
* All paths begin with macros
* All directories are owned by this or other packages
* Make succeeds even when %{_smp_mflags} is defined

Must Fix:
* Package should have ownership of %{_libdir}/xchat

Minor:
* Not preferred buildroot:
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
* Duplicate BuildRequires: perl (by perl), glib2-devel (by GConf2-devel),
pkgconfig (by GConf2-devel), gtk2-devel (by gtkspell-devel)
* some rpmlint errors that aren't blockers:
 W: xchat incoherent-version-in-changelog 1.2.6.6-8 1:2.6.6-8.fc7
 E: xchat tag-not-utf8 %changelog
 W: xchat non-conffile-in-etc /etc/gconf/schemas/apps_xchat_url_handler.schemas

Comment 2 Kevin Kofler 2007-05-27 11:06:30 UTC
Package Change Request
======================
Package Name: xchat
Updated Fedora Owners: caillon, Fedora

Remi Collet wants to comaintain X-Chat:
https://www.redhat.com/archives/fedora-devel-list/2007-May/msg01226.html
and the current owner Christopher Aillon is OK with it:
https://www.redhat.com/archives/fedora-devel-list/2007-May/msg01204.html

Comment 3 Kevin Kofler 2007-05-27 14:03:00 UTC
Package Change Request
======================
Package Name: xchat
Updated Fedora Owners: caillon, Fedora, 
Kevin.org

Change of plan: Remi suggests that I should be a third comaintainer, and I'm OK 
with that, so please add me too while you are at it. ;-) Christopher Aillon 
said in the mail I quoted above that he'd be OK with any comaintainer, so I 
guess he won't object to 2 comaintainers either. ;-)

Comment 4 Kevin Kofler 2007-05-27 15:15:51 UTC
> Must Fix:
> * Package should have ownership of %{_libdir}/xchat

Matthias Clasen fixed this in 1:2.6.6-9. Brian, can he consider this 
approved/resolved now?

Comment 5 Kevin Kofler 2007-05-27 15:16:44 UTC
I mean: Brian, can _we_ consider this approved/resolved now?

Comment 6 Kevin Kofler 2007-06-19 21:58:11 UTC
Brian Pepple: Ping?

> > Must Fix:
> > * Package should have ownership of %{_libdir}/xchat
>
> Matthias Clasen fixed this in 1:2.6.6-9. Brian, can we consider this 
> approved/resolved now?

Those 2 minor issues have also been fixed in the meantime:

> * Not preferred buildroot:
> %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

> E: xchat tag-not-utf8 %changelog

Comment 7 Brian Pepple 2007-06-19 23:00:14 UTC
I should have some time to look at this again tomorrow.

Comment 8 Brian Pepple 2007-06-21 02:11:25 UTC
Couple items still need to be fixed:
1. The spec doesn't install the desktop file, even though it ships one, which is
a Must item.  refer to:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-d559ee7363418a5840ce63090c608c991cd39ce6
2. the desktop file patch (#10) contains some unnecessary (X-Red-Hat-Extras) and
depreciated categories (Application), that will cause the build to error out in
F8 due to the more recent version of desktop-file-utils available there.

Comment 9 Kevin Kofler 2007-06-21 03:42:49 UTC
> 1. The spec doesn't install the desktop file, even though it ships one, which
> is a Must item.

Ugh, blame upstream for that one, the patch just changes the contents of the 
upstream .desktop file. I guess we'll have to work around that. I'll take care 
of that.

> 2. the desktop file patch (#10) contains some unnecessary (X-Red-Hat-Extras)
> and depreciated categories (Application), that will cause the build to error
> out in F8 due to the more recent version of desktop-file-utils available
> there.

Application is again upstream's fault, X-Red-Hat-Extras wasn't in _my_ .desktop 
file which I was forced to revert...

I'll fix the categories in the -redhat-desktop patch.

Comment 11 Brian Pepple 2007-06-22 01:14:37 UTC
Looks like all the blockers I identified have been fixed, so I'll go ahead and
mark this review appropriately.

btw, Kevin, you may want to change the '--vendor fedora' to --vendor="", since
this is a pre-existing package in Fedora, and by adding the vendor you'll be
breaking menu-editing for existing users since it's based off of the desktop
file/path names. 

Comment 12 Kevin Kofler 2007-06-22 01:19:47 UTC
I don't think I can install a .desktop file to itself. I'd have to remove the 
file as installed by upstream and reinstall it from the source tree. Is it 
really worth all that trouble, and also violating the guideline that .desktop 
files are supposed to have a valid vendor? I'm not going to push the .desktop 
file change to F7 updates (and I hope Christopher Aillon and Remi Collet aren't 
going to do that either) because this would break things, but for F7->F8, this 
kind of changes should be expected. I've seen other packages renaming .desktop 
files even in an update.

Comment 13 Brian Pepple 2007-06-22 01:27:01 UTC
Actually, it is in the guidelines that you shouldn't change the vendor_id during
the life of the package.  Since this package never set the vendor_id previously
you want to make sure that is remain unset, hence setting it to ''.  Refer to
the last two lines in desktop-file-install usage in the package guidelines.

Comment 14 Kevin Kofler 2007-06-22 14:15:16 UTC
I know, the guidelines contradict themselves...
> If upstream uses <vendor_id>, leave it intact, otherwise use fedora as 
<vendor_id>. 
> It is important that vendor_id stay constant for the life of a package.
Now which is it?

And what's the point of having merge reviews if we decide to keep existing 
packages the way they are, anyway? But whatever, I'm just going to install it 
with --vendor="" and wait for the bug reports about not honoring the first of 
the 2 contradictory lines coming in... :-(

Comment 15 Kevin Kofler 2007-06-22 14:41:24 UTC
The .desktop file name is bug-for-bug-compatible now:
https://www.redhat.com/archives/fedora-extras-commits/2007-June/msg04167.html
Successfully built for Rawhide.

Comment 16 Rex Dieter 2007-06-22 14:57:50 UTC
imo, "life of package" probably should say "life of package in each 
branch/Fedora-release".   Clear as mud?

Comment 17 Brian Pepple 2007-06-22 15:00:40 UTC
(In reply to comment #14)
> I know, the guidelines contradict themselves...
> > If upstream uses <vendor_id>, leave it intact, otherwise use fedora as 
> <vendor_id>.

This rule refer to *NEW* packages in Fedora.

> > It is important that vendor_id stay constant for the life of a package.
> Now which is it?

If the package is already in Fedora you cannot change the vendor, which is the
case with xchat.

Comment 18 Kevin Kofler 2007-06-22 15:04:35 UTC
IMHO fixing this sort of things is really what new releases are for. I don't 
think bug-for-bug compatibility is a good idea. I've also seen other packages 
changing the vendor name before.

But it's back to the wrong name now in 2.8.2-12.fc8.

Comment 19 Kevin Kofler 2007-07-17 05:59:23 UTC
Package Change Request
======================
Package Name: xchat
Updated Fedora Owners: 
caillon,Fedora,kevin.org

Please change my e-mail address to lowercase, I changed it in the Fedora 
Account System to match Bugzilla, which now normalizes all e-mail addresses to 
lowercase.

Comment 20 Kevin Fenzi 2007-07-17 06:11:54 UTC
cvs done.