Bug 957415 - Review Request: lnav - A curses-based tool for viewing and analyzing log files
Summary: Review Request: lnav - A curses-based tool for viewing and analyzing log files
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Petr Šabata
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-04-28 03:29 UTC by Christopher Meng
Modified: 2014-04-02 11:57 UTC (History)
3 users (show)

Fixed In Version: lnav-0.5.1-2.fc19
Clone Of:
Environment:
Last Closed: 2013-05-23 12:41:28 UTC
Type: ---
Embargoed:
psabata: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Christopher Meng 2013-04-28 03:29:07 UTC
Spec URL: http://cicku.me/lnav.spec
SRPM URL: http://cicku.me/lnav-0.5.0-1.fc20.src.rpm

Description: The log file navigator, lnav, is an enhanced log file viewer that
takes advantage of any semantic information that can be gleaned from
the files being viewed, such as timestamps and log levels.  Using this
extra semantic information, lnav can do things like interleaving
messages from different files, generate histograms of messages over
time, and providing hotkeys for navigating through the file.  It is
hoped that these features will allow the user to quickly and
efficiently zero in on problems.

Fedora Account System Username: cicku

Comment 1 Christopher Meng 2013-04-30 05:42:50 UTC
Koji success:

http://koji.fedoraproject.org/koji/taskinfo?taskID=5316646

Comment 2 Petr Šabata 2013-04-30 16:13:05 UTC
0. The SRPM used in koji build referenced in Comment #1 differs from the one submitted for review.  The one submitted here doesn't build.


1. Similarly to 'lookat', you'll need to regenerate upstream build files to support aarch64.  However, calling autoreconf here fails on automake because some of the standard distribution files, namely AUTHORS and ChangeLog, are missing.

Option #1: Ask upstream to include those files.

Option #2: Force automake to use different strictness level, e.g. "foreign".  See automake info page for details on this.


2. None of the source files mentions their license, neither do the README files.  Both standard BSD 2-clause (./LICENSE) license and GPLv3 (./COPYING) are included.  It's possible the project uses them both but please ask upstream for clarification.  If both are really used, the correct License tag will be "BSD and GPLv3+".  See the Licensing Guidelines for valid short names:
https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#SoftwareLicenses


3. Build-time dependencies are incorrect.
You can safely drop those:
  - glibc-devel
  - libstdc++-devel
  - libtool
However, the following is necessary for the package to build:
  - openssl-devel


4. There's a test suite.  Use it. (i.e., make a %check section which runs "make check")


5. Package COPYING in %doc as well, if relevant (after your resolve issue #2).


6. Summary -- Add "The".


7. %description -- Replace both "lnav" occurences with %{name}.

Comment 3 Christopher Meng 2013-05-01 02:18:40 UTC
(In reply to comment #2)
> 0. The SRPM used in koji build referenced in Comment #1 differs from the one
> submitted for review.  The one submitted here doesn't build.

Oh..Maybe I submitted the wrong revision...Fixed.

> 
> 1. Similarly to 'lookat', you'll need to regenerate upstream build files to
> support aarch64.  However, calling autoreconf here fails on automake because
> some of the standard distribution files, namely AUTHORS and ChangeLog, are
> missing.
> 
> Option #1: Ask upstream to include those files.
> 
> Option #2: Force automake to use different strictness level, e.g. "foreign".
> See automake info page for details on this.

I've sent a mail to upstream and I'm just waiting for the answer.
 
> 2. None of the source files mentions their license, neither do the README
> files.  Both standard BSD 2-clause (./LICENSE) license and GPLv3 (./COPYING)
> are included.  It's possible the project uses them both but please ask
> upstream for clarification.  If both are really used, the correct License
> tag will be "BSD and GPLv3+".  See the Licensing Guidelines for valid short
> names:
> https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#SoftwareLicenses

Sent.

> 3. Build-time dependencies are incorrect.
> You can safely drop those:
>   - glibc-devel
>   - libstdc++-devel
>   - libtool
> However, the following is necessary for the package to build:
>   - openssl-devel

Yeah I know the openssl.a and that's why you failed... This is listed in my koji build, please abandon comment 1~

> 4. There's a test suite.  Use it. (i.e., make a %check section which runs
> "make check")

Fixed.

> 5. Package COPYING in %doc as well, if relevant (after your resolve issue
> #2).

Okay.

> 6. Summary -- Add "The".

Replaced with : A curses-based tool for viewing and analyzing log files

> 7. %description -- Replace both "lnav" occurences with %{name}.

Oh... MUST/SHOULD?

BTW I'll realease next version when everything looks fine.

Comment 4 Petr Šabata 2013-05-02 08:18:44 UTC
I think you forgot to upload the updated version.

Comment 5 Christopher Meng 2013-05-02 10:30:06 UTC
(In reply to comment #4)
> I think you forgot to upload the updated version.

No I have to wait for upstream's licensing answer.

Comment 6 Petr Šabata 2013-05-02 11:22:17 UTC
Alright.

Comment 7 Christopher Meng 2013-05-03 01:51:06 UTC
(In reply to comment #6)
> Alright.

Upstream said that the GPL you saw is the comments generated by autotools.

License is BSD with no doubt.

I've patched the configure.ac but I don't know if it's correct.

New SPEC: http://cicku.me/lnav.spec

New SRPM: http://cicku.me/lnav-0.5.0-2.fc20.src.rpm

Comment 8 Christopher Meng 2013-05-03 16:20:06 UTC
(In reply to comment #6)
> Alright.

Upstream just release 0.5.1 with everything fixed.

New SPEC: http://cicku.me/lnav.spec
New SRPM: http://cicku.me/lnav-0.5.1-1.fc20.src.rpm

Comment 9 Petr Šabata 2013-05-09 12:34:04 UTC
Ok, looks almost perfect now :)

Patching is no longer necessary, you can drop those lines.
And change 'make test' to 'make check'.

Comment 10 Christopher Meng 2013-05-09 13:10:43 UTC
(In reply to comment #9)

Fixed.

New SPEC: http://cicku.me/lnav.spec
New SRPM: http://cicku.me/lnav-0.5.1-2.fc20.src.rpm

Comment 11 Petr Šabata 2013-05-09 13:21:54 UTC
Awesome, approving.

Comment 12 Christopher Meng 2013-05-09 13:28:53 UTC
(In reply to comment #11)
> Awesome, approving.

Thanks!

If you have time please help review some other packages like nsnake (https://bugzilla.redhat.com/show_bug.cgi?id=955913) ,it's really simple with less than 40 lines spec...and monitorix (written in perl)

Comment 13 Christopher Meng 2013-05-09 13:29:27 UTC
New Package SCM Request
=======================
Package Name: lnav
Short Description: A curses-based tool for viewing and analyzing log files
Owners: cicku
Branches: f18 f19
InitialCC:

Comment 14 Gwyn Ciesla 2013-05-09 13:50:20 UTC
Git done (by process-git-requests).

Comment 15 Fedora Update System 2013-05-09 14:18:08 UTC
lnav-0.5.1-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/lnav-0.5.1-2.fc19

Comment 16 Fedora Update System 2013-05-09 14:30:18 UTC
lnav-0.5.1-2.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/lnav-0.5.1-2.fc18

Comment 17 Fedora Update System 2013-05-10 04:53:32 UTC
lnav-0.5.1-2.fc18 has been pushed to the Fedora 18 testing repository.

Comment 18 Fedora Update System 2013-05-23 12:41:28 UTC
lnav-0.5.1-2.fc18 has been pushed to the Fedora 18 stable repository.

Comment 19 Fedora Update System 2013-05-24 20:33:52 UTC
lnav-0.5.1-2.fc19 has been pushed to the Fedora 19 stable repository.

Comment 20 Christopher Meng 2014-04-02 04:27:39 UTC
Package Change Request
======================
Package Name: lnav
New Branches: el6 epel7
Owners: cicku

Comment 21 Gwyn Ciesla 2014-04-02 11:57:45 UTC
Git done (by process-git-requests).


Note You need to log in before you can comment on or make changes to this bug.