Bugzilla will be upgraded to version 5.0 on a still to be determined date in the near future. The original upgrade date has been delayed.
Bug 241075 - Review Request: redet - Regular expression development and execution tool
Review Request: redet - Regular expression development and execution tool
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On:
Blocks: 241076
  Show dependency treegraph
 
Reported: 2007-05-23 18:07 EDT by Nigel Jones
Modified: 2007-11-30 17:12 EST (History)
0 users

See Also:
Fixed In Version: 8.22-4.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-06-09 15:20:14 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Nigel Jones 2007-05-23 18:07:09 EDT
Spec URL: http://dev.nigelj.com/SRPMS/redet.spec
SRPM URL: http://dev.nigelj.com/SRPMS/redet-8.22-1.src.rpm

Description:
Redet is a tool for developing and executing regular expressions using 
any of more than 50 search programs, editors, and programming languages, 
intended both for developing regular expressions for use elsewhere and 
as a search tool in its own right. 

Additional features include persistent history, extensive help, a 
variety of character entry tools, and the ability to change locale while 
running. 

rpmlint is clean, builds in mock and is tested.
Comment 1 Jason Tibbitts 2007-05-31 22:36:29 EDT
Indeed, this builds fine and rpmlint is nice and quiet.

However, I'm not sure why you gzip the manpage and then patch up the Makefile so
that it will install the compressed version.  rpmbuild will compress the
manpages for you; you just have to list them in your %files section like:
 %{_mandir}/man1/redet.1*
Of course, it seems that you need to patch the Makefile for other reasons, which
is fine.  I guess you could just ignore the Makefile and just install the two
files by hand; that would be up to you.

Finally, there's no .desktop file, so this package won't show up in the desktop
menus.  According to the guidelines, every graphical application needs to have a
.desktop file.

* source files match upstream:
   17ca181886030ecba8a1fb7d8906016dd8754f345a22fc3f56817683e93f5342
   redet-8.22.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper (none).
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* rpmlint is silent.
* final provides and requires are sane:
   redet = 8.22-1.fc7
  =
   /bin/sh
   iwidgets
* %check is not present; no test suite upstream.  Seems to run fine for me, 
  although I didn't test extensively.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is already in a separate package.
X GUI app, but no .desktop file.
Comment 2 Nigel Jones 2007-05-31 23:40:08 EDT
(In reply to comment #1)
> Indeed, this builds fine and rpmlint is nice and quiet.
> 
> However, I'm not sure why you gzip the manpage and then patch up the Makefile so
> that it will install the compressed version.  rpmbuild will compress the
> manpages for you; you just have to list them in your %files section like:
>  %{_mandir}/man1/redet.1*
> Of course, it seems that you need to patch the Makefile for other reasons, which
> is fine.  I guess you could just ignore the Makefile and just install the two
> files by hand; that would be up to you.
I'll take a look tonight, regarding this and fix it
> 
> Finally, there's no .desktop file, so this package won't show up in the desktop
> menus.  According to the guidelines, every graphical application needs to have a
> .desktop file.
Another DOH moment, will be fixed when I get home in a few hours.
Comment 3 Nigel Jones 2007-06-01 08:17:15 EDT
Hi Jason,

Spec URL: http://dev.nigelj.com/SRPMS/redet.spec
SRPM URL: http://dev.nigelj.com/SRPMS/redet-8.22-2.src.rpm

I'll leave the man stuff till next upstream release if thats okay with you.

Also, I realise the .desktop file is currently iconless, I'm currently in the
search for one thats up to standard (I checked out Debian and they seem to be
iconless, I might see if anyone else has adapted one before I decide to do a not
very good one ;))
Comment 4 Jason Tibbitts 2007-06-04 15:07:48 EDT
I recall that you were going to just use the same icon as kregexpeditor.  Do you
want me to wait for that package before finishing this up?
Comment 5 Nigel Jones 2007-06-04 16:03:37 EDT
(In reply to comment #4)
> I recall that you were going to just use the same icon as kregexpeditor.  Do you
> want me to wait for that package before finishing this up?

It's up to you, but I can have an updated package out in about 16 hours. (I
might see if I can do it before I head out though)
Comment 6 Nigel Jones 2007-06-04 16:15:51 EDT
(In reply to comment #5)
> (In reply to comment #4)
> > I recall that you were going to just use the same icon as kregexpeditor.  Do you
> > want me to wait for that package before finishing this up?
> 
> It's up to you, but I can have an updated package out in about 16 hours. (I
> might see if I can do it before I head out though)
It didn't take as long as I thought it would...

Spec URL: http://dev.nigelj.com/SRPMS/redet.spec
SRPM URL: http://dev.nigelj.com/SRPMS/redet-8.22-3.src.rpm
Comment 7 Jason Tibbitts 2007-06-07 23:02:36 EDT
Sorry it's taken me three days to get back to this.

It's fine if you put off the manpage bit; it's kind of pointless to compress it
yourself but I don't think it's a blocker.

Unfortunately this fails to build in mock for me; you just need to add a build
dependency on desktop-file-utils and everything goes through fine.

I installed on my desktop and found a problem with the desktop file: It has no
GenericName entry, so instead of "Redet (Regular expression whatever)" I just
get "Redet".  I would recommend replacing "Comment" with "GenericName" in your
desktop file, but the desktop entry spec doesn't require GenericName so I don't
think it's a blocker.

I am not sure if TCL actually supports StartupNotify=true, but startup
notification seems to work well enough for me under KDE so I suppose it's OK.

So since there's only one remaining trivial blocker and it will keep you from
actually building the package, I'll go ahead and approve this and you can fix it
when you check in.

APPROVED
Comment 8 Nigel Jones 2007-06-08 00:01:01 EDT
(In reply to comment #7)
> Sorry it's taken me three days to get back to this.
> 
> It's fine if you put off the manpage bit; it's kind of pointless to compress it
> yourself but I don't think it's a blocker.
> 
> Unfortunately this fails to build in mock for me; you just need to add a build
> dependency on desktop-file-utils and everything goes through fine.
Ahh! I'll fix that when I get home.
> 
> I installed on my desktop and found a problem with the desktop file: It has no
> GenericName entry, so instead of "Redet (Regular expression whatever)" I just
> get "Redet".  I would recommend replacing "Comment" with "GenericName" in your
> desktop file, but the desktop entry spec doesn't require GenericName so I don't
> think it's a blocker.
I'll fix this tonight as well.
> 
> I am not sure if TCL actually supports StartupNotify=true, but startup
> notification seems to work well enough for me under KDE so I suppose it's OK.
> 
> So since there's only one remaining trivial blocker and it will keep you from
> actually building the package, I'll go ahead and approve this and you can fix it
> when you check in.
> 
> APPROVED
New Package CVS Request
=======================
Package Name: redet
Short Description: Regular expression development and execution tool
Owners: dev@nigelj.com
Branches: FC-6 F-7 EL-4 EL-5
InitialCC: 
Comment 9 Fedora Update System 2007-06-09 15:20:12 EDT
redet-8.22-4.fc7 has been pushed to the Fedora 7 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.