Bug 884759 - Review Request: toilet - colorful ASCII art generator
Review Request: toilet - colorful ASCII art generator
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On: 1062632
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2012-12-06 11:43 EST by Zuzana
Modified: 2016-02-08 09:06 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-02-08 09:06:40 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Zuzana 2012-12-06 11:43:24 EST
Spec URL: http://jnovy.fedorapeople.org/toilet/toilet.spec
SRPM URL: http://jnovy.fedorapeople.org/toilet/toilet-0.2-1.fc18.src.rpm

Description: 

The TOIlet project attempts to create a free replacement for the FIGlet
utility. TOIlet stands for "The Other Implementation's letters", coined
after FIGlet's "Frank, Ian and Glen's letters".

TOIlet is in its very early development phase. It uses the powerful
libcaca library to achieve various text-based effects. TOIlet
implements or plans to implement the following features:

    The ability to load FIGlet fonts
    Support for Unicode input and output
    Support for colour fonts
    Support for colour output
    Support for various output formats: HTML, IRC, ANSI... 

TOIlet also aims for full FIGlet compatibility. It is currently able to
load FIGlet fonts and perform horizontal smushing.
Fedora Account System Username:
Comment 1 Michael Scherer 2012-12-07 20:50:57 EST
Hi,

you didn't gave your username, and the email you us do not correspond to the one of jnovy in FAS, can you give the info ?
Comment 2 Fabian Affolter 2012-12-08 08:38:43 EST
Just some quick comments:

- The latest version of toilet is 0.3
- Please rephrase the summery
- There is a test folder in the source tarball. Please consider to run the test in a %check section
- There are fonts in the source tarball. Are those fonts bundled?
Comment 3 Zuzana 2012-12-11 07:22:12 EST
- FAS username is sysl
Comment 4 Zuzana 2012-12-11 08:29:16 EST
 - toilet 0.3 build requires new libcaca library still not packaged in Fedora.
Comment 5 Michael Scherer 2012-12-11 09:43:00 EST
Since you are not in any packager group, could you please follow the step on  https://fedoraproject.org/wiki/Join_the_package_collection_maintainers ?

I have marked this bug as needing a sponsor.
Comment 7 Jindrich Novy 2012-12-11 14:53:28 EST
Taking this review.
Comment 8 Jindrich Novy 2012-12-11 15:12:31 EST
rpmlint output on src.rpm:

$ rpmlint toilet-0.2-0.2.fc18.src.rpm 
toilet.src: W: spelling-error Summary(en_US) figlet -> filet, fillet, piglet
-> sane as it references figlet utility

toilet.src: W: summary-not-capitalized C figlet alternative
-> could you please rephrase the Summary in a way that it starts with a capital letter?

toilet.src: W: spelling-error %description -l en_US ancronym -> acronym, antonym
-> there seems to be a typo in %description here

toilet.src: W: spelling-error %description -l en_US libcaca -> biblical
-> sane, it's a library name

toilet.src: W: spelling-error %description -l en_US folowing -> flowing, following, fol owing
-> seems that 'l' is missing in "following"

toilet.src: W: spelling-error %description -l en_US colour -> color, co lour, co-lour
-> US English prefers "color"

toilet.src: W: spelling-error %description -l en_US smushing -> mushing, smashing, shushing
-> not sure here but 

IMO the %description seems to me too verbose with a lot of duplicities. Could you please reduce it?

Running rpmlint on built RPM reveals:
toilet.x86_64: E: standard-dir-owned-by-package /usr/share/man/man1

So it seems it is better to own the man page like this in the %files section:
%{_mandir}/man1/%{name}*
Comment 9 Michael Schwendt 2012-12-14 07:40:24 EST
> > Summary:        figlet alternative

> could you please rephrase the Summary in a way that it starts with
> a capital letter?

Even better, replace the summary with a clear and concise one. Such as the one used for the title of this ticket:

  Summary: Colorful ASCII Art Generator

Taking the summary from the manual page, it would be:

  Summary: Display large colorful characters

Referencing the "figlet" utility might be sane in the longer description, although it requires the reader to know the "figlet" utility. In the summary, however, it should be possible to "sum up" what a package does without referring to the name of some other package or piece of software. Don't focus on what the package/software is named. The package %name is for that, and the %description can also expand on the name of the software.


> %{_datadir}/figlet/*

https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
https://fedoraproject.org/wiki/Packaging:UnownedDirectories
Comment 11 Christopher Meng 2013-09-05 09:49:06 EDT
Status?
Comment 12 Christopher Meng 2013-09-05 09:49:59 EDT
What's up?
Comment 13 Christopher Meng 2013-11-12 10:30:23 EST
ping again.
Comment 14 Zuzana 2014-02-06 13:46:33 EST
Packages updated. Thank you for advice and sorry for the delay. 

http://jnovy.fedorapeople.org/toilet/toilet.spec
http://jnovy.fedorapeople.org/toilet/toilet-0.2-1.fc19.src.rpm
Comment 15 Christopher Meng 2014-02-06 21:21:17 EST
(In reply to Zuzana from comment #14)
> Packages updated. Thank you for advice and sorry for the delay. 

No problem. Issues:

1. The latest version is 0.3. Please read comment 3 carefully.

2. Remove rm -rf $RPM_BUILD_ROOT in %install, not needed anymore nowadays.

3. And, you didn't bump the changelog for each change, please do from now on.

4. Michael pointed out the poor summary in the past, and I think the title of this bug contains a good example, however the one you currently use is poor still:

5. Summary: Display large colorful characters is still not perfect.

--------One day--------

Noob A: Is it a library?
Noob B: Maybe, but I think it's a utility, bash script?
Noob C: Why bash script? Can be javascript as well!
Noob D: W** is the "large colorful"?
Noob E: Dunno.

Then they go away.

-----------------------

Advice: Since you still need a sponsor, please send a self introduction email to @devel list to introduce yourself.

To assignee: Please keep review flag cohering with the assignee, please re assign this to yourself if you still have interests.
Comment 16 Zuzana 2014-02-07 08:51:26 EST
(In reply to Christopher Meng from comment #15)


> 1. The latest version is 0.3. Please read comment 3 carefully.

Toilet 0.3 requires libcaca library at least in version 0.99.beta18 but it is not available in any Fedora. Therefore such update is not possible and we need to stick to 0.2:

configure: error: you need libcaca version 0.99.beta18 or later
error: Bad exit status from /var/tmp/rpm-tmp.G43QWj (%build)


> 2. Remove rm -rf $RPM_BUILD_ROOT in %install, not needed anymore nowadays.

Done, thanks. 

> 3. And, you didn't bump the changelog for each change, please do from now on.

Changelog entry added and release bumped.

> 4. Michael pointed out the poor summary in the past, and I think the title
> of this bug contains a good example, however the one you currently use is
> poor still:
> 5. Summary: Display large colorful characters is still not perfect.

Summary is now changed to "Colorful ASCII Art Generator".

> -----------------------
> 
> Advice: Since you still need a sponsor, please send a self introduction
> email to @devel list to introduce yourself.
> 
> To assignee: Please keep review flag cohering with the assignee, please re
> assign this to yourself if you still have interests.
Comment 17 Christopher Meng 2014-02-07 09:03:30 EST
Then you need to report a bug against libcaca via:

https://bugzilla.redhat.com/enter_bug.cgi?product=Fedora&version=rawhide&component=libcaca
Comment 18 Miroslav Suchý 2015-07-21 09:46:36 EDT
Ping! Any progress here?
Comment 19 Miroslav Suchý 2016-02-08 09:06:40 EST
No response. Closing as dead review. If you ever want to continue, please resubmit.

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