Bug 225669

Summary: Merge Review: ctags
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Terje Røsten <terje.rosten>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: than
Target Milestone: ---Flags: terje.rosten: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-07-29 15:42:51 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:
Attachments:
Description Flags
patch to ctags.spec to include ctage-etags package. none

Description Nobody's working on this, feel free to take it 2007-01-31 17:54:07 UTC
Fedora Merge Review: ctags

http://cvs.fedora.redhat.com/viewcvs/devel/ctags/
Initial Owner: than

Comment 1 Terje Røsten 2007-07-02 20:13:35 UTC
+ rpmlint
W: ctags summary-ended-with-dot A C programming language indexing and/or
cross-reference tool.
E: ctags tag-not-utf8 %changelog
E: ctags non-utf8-spec-file ctags.spec
W: ctags summary-ended-with-dot A C programming language indexing and/or
cross-reference tool.
E: ctags tag-not-utf8 %changelog
E: ctags-debuginfo tag-not-utf8 %changelog

Comment:
 * fix utf-8 with iconv(1)
 * Both Summary and description should be reworked, they don't match the package
as of
   today.

+ naming : ok

+ guidelines
 * changelog is 10 years old (sic!), could be a be shorter?
 * disttag is missing
 * source url is wrong, see:
  
http://fedoraproject.org/wiki/Packaging/SourceURL#head-e27982f18a3bfd26b5b6ecbee113d2d8f3f006f2
 * I prefer this buildroot:
   %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
 * add  %{?_smp_mflags} to make
 * preserve timestamp on man files (cp -p/install -p)
 * don't use %makeinstall
  
http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002
 * don't shiop INSTALL
 the rest seems ok.

+ License: ok
+ American English: ok, however as noted the summary and description is outdated
and should be fixed.
+ legible: ok
+ src: md54sum: 9026a6c6950751bc4fd1be37e8a2070f : ok
+ build: ok (with correct build opts)
+ buildreq: ok
+ dirs and dups: ok
+ I prefer %defattr(...) as %defattr(-, root, root, -)
+ clean: ok
+ debuginfo: ok
Then a issue maybe outside the scope of this review, I really like to see etags
enabled and
included. Perhaps in a ctags-etags subpackage like this:

%package etags
Summary: Exuberant Ctags for emacs tag format
Group: Development/Tools
Requires: ctags = %{version}-%{release}
%description
This package will generate tags in a format which GNU Emacs understand,
it's a alternativ implementation of the GNU etags program.
Note: some command line options is not compatible with GNU etags.

Then change %configure line to include --enable-etags .

There are several other /usr/bin/etags coming from emacs and xemacs,
hence you must use alternatives to setup /usr/bin/etags.ctags as
alternativ. I can help with the details if you want, it goes
something like this:

move files (after install):

mv /usr/bin/etags to /usr/bin/etags.ctags
mv /usr/share/man/man1/etags.1 /usr/share/man/man1/etags.ctags.1

set up alternatives with slave (in %post and %preun):

alternatives --install /usr/bin/etags etags /usr/bin/etags.ctags 20 \
 --slave /usr/share/man/man1/etags.1.gz etags-etagsman
/usr/share/man/man1/etags.ctags.1.gz

alternatives --remove etags /usr/bin/etags.ctags

With low prio as 20, xemacs and emacs should ne affected.
and included the files with

%files etags
%defattr(-, root, root, -)
%doc COPYING
%{_bindir}/etags.ctags
%{_mandir}/man1/etags*






Comment 2 Terje Røsten 2008-01-14 20:36:09 UTC
ping?

Comment 3 Than Ngo 2008-02-15 16:26:40 UTC
>Comment:
> * fix utf-8 with iconv(1)
fixed

> * Both Summary and description should be reworked, they don't match the 
>package as of today.

what is the problem here? could you please give more infos?

>+ guidelines
> * changelog is 10 years old (sic!), could be a be shorter?
the old changelog should be remained
 
>* disttag is missing
it's fixed

> * source url is wrong, see:
>http://fedoraproject.org/wiki/Packaging/SourceURL#head-e27982f18a3bfd26b5b6ecbee113d2d8f3f006f2
 
it's fixed

>* I prefer this buildroot:
>   %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)

i prefer %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

> * add  %{?_smp_mflags} to make
fixed

> * preserve timestamp on man files (cp -p/install -p)

fixed

> * don't use %makeinstall
 fixed
 
>http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002
> * don't shiop INSTALL
>the rest seems ok.
fixed

>+ License: ok
>+ American English: ok, however as noted the summary and description is 
>outdated
>and should be fixed.

any idea how to fix it?

>Then a issue maybe outside the scope of this review, I really like to see 
>etags
>enabled and
>included. Perhaps in a ctags-etags subpackage like this:
>
it doesn't make sense to add redundant etags here.
wont fix


i built new ctags-5.7 in rawhide. could you please check again. Thanks


Comment 4 Terje Røsten 2008-02-15 20:57:08 UTC
( > * Both Summary and description should be reworked, they don't match the 
> >package as of today.
> 
> what is the problem here? could you please give more infos?

The summary and description only talks about C code, however
ctags now supports a lot of program languages:

http://ctags.sourceforge.net/languages.html

This fact should be stated in the summary and description, from the the website.

Something along the lines of the rpm from the website maybe:

Summary:

Exuberant Ctags - a multi-language source code indexing tool

Desc:

Exuberant Ctags generates an index (or tag) file of language objects
found in source files for many popular programming languages. This index
makes it easy for text editors and other tools to locate the indexed
items. Exuberant Ctags improves on traditional ctags because of its
multilanguage support, its ability for the user to define new languages
searched by regular expressions, and its ability to generate emacs-style
TAGS files.

Install ctags if you are going to use your system for programming.



> the old changelog should be remained

Ok.
 
> i prefer %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Fine.
 
> >+ License: ok

The guidelines has changed since the initial review, the license is in fact not
valid, please have a  look at this issue.

> >+ American English: ok, however as noted the summary and description is 
> >outdated
> >and should be fixed.
> 
> any idea how to fix it?

See over?
 
> >Then a issue maybe outside the scope of this review, I really like to see 
> >etags
> >enabled and
> >included. Perhaps in a ctags-etags subpackage like this:
> >
> it doesn't make sense to add redundant etags here.
> wont fix

I don't see the problem in including a ctags-etags subpackage for users that
will swap GNU Emacs / XEmacs etags for Exuberant Ctags etags.

I have created a ctags package with a ctags-etags subpackage. The correct
alternatives commands are used. A patch is included as attachment.

Please reconsider for inclusion.




Comment 5 Terje Røsten 2008-02-15 21:00:25 UTC
Created attachment 295038 [details]
patch to ctags.spec to include ctage-etags package.

patch for comment #4.

Comment 6 Debarshi Ray 2008-07-26 08:17:02 UTC
Ping?

Comment 7 Than Ngo 2008-07-29 15:42:51 UTC
it's fixed in rawhide. thanks

Comment 8 Jason Tibbitts 2008-08-14 13:51:09 UTC
Terje: Did you intend to approve this ticket?  It's closed but fedora-review is still set to '?'.

Comment 9 Terje Røsten 2008-09-01 18:51:08 UTC
Seems like all issues are fixed now (etags subpackage and license).

The Summary and  Description texts can be improved, however the package
can be approved as is.

But as you say tibbs, I don't know why the packager closed the ticket before the package was approved.