Bug 885833 - Review Request: tw - translate words into different languages
Review Request: tw - translate words into different languages
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Rex Dieter
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-10 13:15 EST by Deleted Account
Modified: 2013-05-01 17:24 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-12-27 22:48:52 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rdieter: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)
tw-0.9.4-4.spec (2.43 KB, application/octet-stream)
2012-12-10 17:58 EST, Deleted Account
no flags Details

  None (edit)
Description Deleted Account 2012-12-10 13:15:14 EST
This is my first package.

Spec URL: http://translateword.googlecode.com/files/tw.spec
SRPM URL: http://translateword.googlecode.com/files/tw-0.9.4-0.1.fc17.src.rpm
Description: command line language translator (http://code.google.com/p/translateword)
Fedora Account System Username: juanmabc
Comment 1 Antonio Trande 2012-12-10 13:40:47 EST
Hi Juan.
There are some issues just for starting:

- License should be GPLv3.

- Source* tag is the full URL for the compressed archive containing the (original) pristine source code.
https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL

- BuildRequires and Requires entries listed one-by-one; it is important for a better spec legibility.  

- In BuildRequires tag, coreutils, bash and sed can be omitted.
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Exceptions_2

- Description is too long for a single line.

- Changelog is missing

For more details, you can start with http://fedoraproject.org/wiki/Packaging:Guidelines
Comment 4 Eduardo Echeverria 2012-12-10 17:16:19 EST
Hi Juan
- Please put in the field Blocks the tag FE-NEEDSPONSOR.
- The spec should be in a place where it can be reviewed online, and not to download.
- the package what provides /usr/bin/xmllint is libxml2
- The tag Release is incorrect, in this case should be only "3", not  "0.3"
Comment 5 Deleted Account 2012-12-10 17:42:02 EST
I'm gonna attach the spec then.
I was thinking about the tag scheme, fixed.
Block tag FE-NEEDSPONSOR updated, rdieter from #fedora-kde offered.

I was already using libxml2 as requires, but rpmlint complains, not with a warning but an error:
tw.i686: E: explicit-lib-dependency libxml2

Please confirm to use Requires: /usr/bin/xmllint or Requires: libxml2, as the later raises an rpmlint error. After that i'll upload.
Comment 6 Deleted Account 2012-12-10 17:58:38 EST
Created attachment 661177 [details]
tw-0.9.4-4.spec
Comment 7 Deleted Account 2012-12-10 18:01:24 EST
As this is a shell script that is calling xmllint, the final way to go seems Requires: %{_bindir}/xmllint (rdieter).

Spec URL: tw-0.9.4-4.spec attached.
SRPM URL: http://translateword.googlecode.com/files/tw-0.9.4-4.fc17.src.rpm
Comment 8 Eduardo Echeverria 2012-12-11 01:23:59 EST
Hi Juan

In "Requires", I consider that this should not go
Requires:       sed
Requires:       bash
Requires:       gawk
Requires:       coreutils

rpm will auto-detect most Requires: based on what your binary links with

- apparently %{_bindir}/xmllint is detected correctly.
- In koji can see 

http://kojipkgs.fedoraproject.org//work/tasks/7249/4777249/build.log

checking for sed... yes
checking for bash... yes
checking for getopt... yes
checking for gawk... yes
checking for curl... yes
checking for iconv... yes
checking for xmllint... yes
checking for lynx... no
checking for aspell... no

lynx and aspell should be BuildRequires

- Perhaps I did not explain, when i say that your spec  "should be in a place where it can be reviewed online, and not to download."  I mean a direct web link,  like this:
http://echevemaster.fedorapeople.org/xfce-theme-manager/1/xfce-theme-manager.spec

Regards
Comment 9 Eduardo Echeverria 2012-12-11 02:50:53 EST
Oh I forgot, the name of your spec should not contain more nothing that the name of the program, without version numbers. i.e is correct "tw.spec" instead of  "tw-0.9.4-4.spec"
Comment 10 Deleted Account 2012-12-11 05:19:18 EST
*
It is not a binary linking to any program or lib (apart from a real executable utility linking to mythes trhough hunspell, keeping it arch specific instead of fully noarch package), it is a shell script binary (#!/usr/bin/env bash).

Anyway, Requires cleaned, lynx and aspell added in Requires:

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

*
Accessing Your fedorapeople.org Space
- You must be sponsored in a group (other than the CLA groups)

I could keep trying to get access to any place like that but if the already provided places don't work i can't seem to be able to provide such space. Try to select "open file" instead of "save/save as" on browser dialog.

*
Spec URL: http://translateword.googlecode.com/files/tw.spec
SRPM URL: http://translateword.googlecode.com/files/tw-0.9.4-5.fc17.src.rpm
Comment 11 Kevin Kofler 2012-12-11 06:50:17 EST
> In "Requires", I consider that this should not go
> Requires:       sed
> Requires:       bash
> Requires:       gawk
> Requires:       coreutils

Uh, no, these are actually needed!

I think bash is only autodetected if you use #!/bin/bash directly rather than #!/usr/bin/env bash. The others are definitely NOT autodetected, because they're only called in the script, not linked by anything nor used as a #! line.
Comment 12 Kevin Kofler 2012-12-11 06:51:03 EST
(and there are NO implicit Requires, unlike BuildRequires, because you cannot make any assumptions about user systems)
Comment 13 Deleted Account 2012-12-11 07:46:06 EST
* Do not assume anything about Requires.
Spec URL: http://translateword.googlecode.com/files/tw.spec
SRPM URL: http://translateword.googlecode.com/files/tw-0.9.4-6.fc17.src.rpm
Comment 14 Rex Dieter 2012-12-13 11:46:18 EST
i can help finish up the review today (hopefully)...
Comment 15 Rex Dieter 2012-12-13 12:03:48 EST
naming: ok

sources: ok
md5sum *.bz2
8e0a7c167e357c860601b5348469ad15  tw-0.9.4.tar.bz2

license: close, but...
licensecheck -r src
src/tw.in.sh: GPL (v3 or later)
src/tw_mythes.sh: GPL (v3 or later)
src/cmd.sh: GPL (v3 or later)
src/mythes.cxx: *No copyright* UNKNOWN
src/engines/tw_cache.sh: GPL (v3 or later)
src/engines/tw_ft.sh: GPL (v3 or later)
src/engines/tw_share.sh: GPL (v3 or later)
src/engines/tw_yb.sh: GPL (v3 or later)
src/engines/tw_local.sh: GPL (v3 or later)
src/engines/tw_gt.sh: GPL (v3 or later)

1. MUST: Seems we can use
License: GPLv3+
here

2. SHOULD consider omitting deprecated stuff from .spec like
BuildRoot:
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#BuildRoot_tag

Group:
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Group_tag

%clean:
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#.25clean

%defattr:
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#File_Permissions

scriptlets: ok

macros: ok

dependencies: NOT ok
3. MUST:  hrm, I can't find it explicitly in the guidelines at the moment, but pretty sure you can safely omit:
Requires: glibc-common
which is already implicitly pulled in by pretty much everything already.


Being not a complicated package, the rest looks good, please look over items 1-3.
Comment 16 Deleted Account 2012-12-13 13:21:17 EST
Very instructive, thanks.

* Mon Dec 13 2012 Juan Manuel Borges Caño <juanmabcmail@gmail.com> - 0.9.4-7
- Use GPLv3+.
- Omit deprecated stuff like BuildRoot, Group, clean and defattr.
- Omit Requires: glibc-common, implicitly pulled by pretty much everything already.
- Add tw-config-klipper to files.

Spec URL: http://translateword.googlecode.com/files/tw.spec
SRPM URL: http://translateword.googlecode.com/files/tw-0.9.4-7.fc17.src.rpm
Comment 17 Rex Dieter 2012-12-13 13:56:57 EST
Looks good, though the tarball changed since comment #15 to include your klipper bit.

As upstream, that's a bit your perogative.


APPROVED (and sponsored).

Move on to:
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner
Comment 18 Deleted Account 2012-12-13 14:10:39 EST
New Package SCM Request
=======================
Package Name: tw
Short Description: Translate words into different languages
Owners: juanmabc
Branches: f17 f18
InitialCC:
Comment 19 Jon Ciesla 2012-12-13 14:31:49 EST
Git done (by process-git-requests).
Comment 20 Deleted Account 2012-12-13 17:03:36 EST
Package Change Request
======================
Package Name: tw
New Branches: f16
Owners: juanmabc

koji build --scratch f16 succeeds.
Comment 21 Fedora Update System 2012-12-13 17:57:55 EST
tw-0.9.4-7.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/tw-0.9.4-7.fc18
Comment 22 Fedora Update System 2012-12-13 17:59:28 EST
tw-0.9.4-7.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/tw-0.9.4-7.fc17
Comment 23 Fedora Update System 2012-12-14 01:49:25 EST
tw-0.9.4-7.fc18 has been pushed to the Fedora 18 testing repository.
Comment 24 Jon Ciesla 2012-12-14 08:41:48 EST
Git done (by process-git-requests).
Comment 25 Fedora Update System 2012-12-14 11:08:28 EST
tw-0.9.4-7.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/tw-0.9.4-7.fc16
Comment 26 Fedora Update System 2012-12-27 22:48:54 EST
tw-0.9.4-7.fc17 has been pushed to the Fedora 17 stable repository.
Comment 27 Fedora Update System 2012-12-27 22:54:05 EST
tw-0.9.4-7.fc16 has been pushed to the Fedora 16 stable repository.
Comment 28 Fedora Update System 2013-01-11 19:11:56 EST
tw-0.9.4-7.fc18 has been pushed to the Fedora 18 stable repository.
Comment 29 Fedora Update System 2013-01-17 14:53:16 EST
tw-0.9.8-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/tw-0.9.8-1.fc18
Comment 30 Fedora Update System 2013-01-17 14:54:02 EST
tw-0.9.8-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/tw-0.9.8-1.fc17
Comment 31 Fedora Update System 2013-01-17 14:54:50 EST
tw-0.9.8-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/tw-0.9.8-1.fc16
Comment 32 Fedora Update System 2013-01-28 09:53:05 EST
tw-0.9.8-1.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 33 Fedora Update System 2013-01-28 10:20:21 EST
tw-0.9.8-1.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 34 Fedora Update System 2013-01-28 10:22:42 EST
tw-0.9.8-1.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 35 Fedora Update System 2013-04-28 15:19:04 EDT
tw-0.9.16-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/tw-0.9.16-1.fc19
Comment 36 Fedora Update System 2013-04-28 15:20:25 EDT
tw-0.9.16-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/tw-0.9.16-1.fc18
Comment 37 Fedora Update System 2013-04-28 15:21:25 EDT
tw-0.9.16-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/tw-0.9.16-1.fc17
Comment 38 Kevin Kofler 2013-04-28 17:00:42 EDT
Please DO NOT reference the review request on every single update of the package. Doing so only makes sense for the initial newpackage update. Once the package is in (i.e. the initial update is pushed), the review request is closed and further updates need not and should not reference it anymore.
Comment 39 Fedora Update System 2013-05-01 17:22:57 EDT
tw-0.9.16-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/tw-0.9.16-2.fc19
Comment 40 Fedora Update System 2013-05-01 17:23:49 EDT
tw-0.9.16-2.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/tw-0.9.16-2.fc18
Comment 41 Fedora Update System 2013-05-01 17:24:31 EDT
tw-0.9.16-2.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/tw-0.9.16-2.fc17

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