Bug 1460246 - Review Request: wordgrinder - A minimal word processor
Review Request: wordgrinder - A minimal word processor
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Gwyn Ciesla
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2017-06-09 09:20 EDT by Ben Cotton
Modified: 2017-07-06 16:02 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2017-07-06 16:02:10 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
limburgher: fedora‑review+


Attachments (Terms of Use)
Build log (21.23 KB, text/plain)
2017-07-06 08:51 EDT, Gwyn Ciesla
no flags Details

  None (edit)
Description Ben Cotton 2017-06-09 09:20:46 EDT
Spec URL: https://bcotton.fedorapeople.org/wordgrinder/wordgrinder.spec
SRPM URL: https://bcotton.fedorapeople.org/wordgrinder/wordgrinder-0.6-1.fc25.src.rpm
Description: WordGrinder is a Unicode-aware character cell word processor that runs in a terminal (or a Windows console). It is designed to get the hell out of your way
and let you get some work done. WordGrinder is a word processor for processing words. It is not WYSIWYG. It is not point and click. It is not a desktop publisher. It is not a text editor. It does not do fonts and it barely does styles. What it does do is words. It's designed for writing text.

Fedora Account System Username: bcotton

rpmlint output:
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
Comment 1 Iwicki Artur 2017-06-10 09:23:19 EDT
>Patch1:         lua_package.patch
While the patch itself is rather straight-forward, it'd still be nice to have a short comment in the spec file explaining what it does.

>Summary:        (...) done.
>Group:          Applications/Productivity
The Summary should not end with a dot and the Group tag should not be used at all.
https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections

>make -j
I believe you should use the %{?_smp_mflags} macro here (it expands to the -j option).

>make install PREFIX=$RPM_BUILD_ROOT/usr
Use the %{_prefix} macro instead of "/usr".

>%_mandir/man1/wordgrinder.1.gz
You should not assume that gzip will be used for compressing manpages - instead, use a pattern that can match both compressed and non-compressed files.
https://fedoraproject.org/wiki/Packaging:Guidelines#Manpages

>* Wed Jun 07 2017 Ben Cotton <bcotton@fedoraporject.org> - 0.6-1
Probably a typo in the domain name ("porject" instead of "project").

Also, there are some rpmlint errors:
wordgrinder.x86_64: E: explicit-lib-dependency libXft
wordgrinder.x86_64: E: explicit-lib-dependency zlib
wordgrinder.x86_64: E: summary-too-long
Comment 2 Ben Cotton 2017-06-10 20:41:49 EDT
Thanks for the quick review. I believe I've addressed all of your concerns. Spec and SRPM updated:

Spec URL: https://bcotton.fedorapeople.org/wordgrinder/wordgrinder.spec
SRPM URL: https://bcotton.fedorapeople.org/wordgrinder/wordgrinder-0.6-1.fc25.src.rpm
Comment 3 Ben Cotton 2017-07-05 20:43:59 EDT
Updated again to address upstream's concerns with the patch:

Spec: https://bcotton.fedorapeople.org/wordgrinder/wordgrinder.spec
SRPM: https://bcotton.fedorapeople.org/wordgrinder/wordgrinder-0.6-2.fc25.src.rpm
Comment 4 Gwyn Ciesla 2017-07-06 08:51 EDT
Created attachment 1294931 [details]
Build log

Fails to build on f26.
Comment 5 Ben Cotton 2017-07-06 11:40:47 EDT
Ah! Looks like luajit changed versions. I've reworked it a little bit and it now builds in f25 and f26 mocks:

Spec: https://bcotton.fedorapeople.org/wordgrinder/wordgrinder.spec
SRPM: https://bcotton.fedorapeople.org/wordgrinder/wordgrinder-0.6-3.fc25.src.rpm
Comment 6 Gwyn Ciesla 2017-07-06 12:32:48 EDT
Much better.

- rpmlint checks return:

wordgrinder-debuginfo.x86_64: E: debuginfo-without-sources
This debuginfo package appears to contain debug symbols but no source files.
This is often a sign of binaries being unexpectedly stripped too early during
the build, or being compiled without compiler debug flags (which again often
is a sign of distro's default compiler flags ignored which might have security
consequences), or other compiler flags which result in rpmbuild's debuginfo
extraction not working as expected.  Verify that the binaries are not
unexpectedly stripped and that the intended compiler flags are used.


- package meets naming guidelines
- package meets packaging guidelines
- license ( MIT ) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file

It would be good to break up the BuildRequires into multiple lines, for readability.

So at the moment the debuginfo is the only fix needed.
Comment 7 Ben Cotton 2017-07-06 14:22:27 EDT
I broke up the BuildRequires as suggested and modified my patch to correct the debuginfo issues.

rpmlint now returns:
wordgrinder-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/wordgrinder-wordgrinder-0.6/.obj
wordgrinder-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/wordgrinder-wordgrinder-0.6/.obj
1 packages and 0 specfiles checked; 0 errors, 2 warnings.


The hidden file is expected. Updated SRPM/spec:

Spec: https://bcotton.fedorapeople.org/wordgrinder/wordgrinder.spec
SRPM: https://bcotton.fedorapeople.org/wordgrinder/wordgrinder-0.6-4.fc25.src.rpm
Comment 8 Gwyn Ciesla 2017-07-06 14:33:15 EDT
Excellent.  There's also:

wordgrinder.src:15: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 15)

but you can finish that at import.

APPROVED.
Comment 9 Gwyn Ciesla 2017-07-06 15:25:16 EDT
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/wordgrinder

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