Bug 960839 - Review Request: langtable - For guessing reasonable defaults for locale, keyboard, territory, …
Summary: Review Request: langtable - For guessing reasonable defaults for locale, keyb...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-05-08 06:15 UTC by Mike FABIAN
Modified: 2013-09-25 03:56 UTC (History)
10 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2013-07-15 04:56:46 UTC
Type: ---
Embargoed:
panemade: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Mike FABIAN 2013-05-08 06:15:49 UTC
Spec URL: http://mfabian.fedorapeople.org/lang-table/lang-table.spec
SRPM URL: http://mfabian.fedorapeople.org/lang-table/lang-table-0.0.1-1.fc18.src.rpm

Description:
lang-table is used to guess reasonable defaults for locale, keyboard,
territory, …, if part of that information is already known. For
example, guess the territory and the keyboard layout if the language
is known or guess the language and keyboard layout if the territory is
already known.

This is intended to be used by anaconda.

Fedora Account System Username: mfabian

Comment 1 Mike FABIAN 2013-05-08 07:46:49 UTC
BuildRequires:  python-lxml
was missing, therefore the %check section always failed for
me when building with “mock”. Jens Petersen found the reason.
Fixed it like as below.

Spec URL: http://mfabian.fedorapeople.org/lang-table/lang-table.spec
SRPM URL: http://mfabian.fedorapeople.org/lang-table/lang-table-0.0.1-1.fc18.src.rpm

updated.

diff --git a/lang-table.spec b/lang-table.spec
index 76f25d0..582bd73 100644
--- a/lang-table.spec
+++ b/lang-table.spec
@@ -9,6 +9,7 @@ License:        GPLv3+
 URL:            https://github.com/mike-fabian/lang-table
 Source0:        http://mfabian.fedorapeople.org/lang-table/%{name}-%{version}.tar.gz
 BuildArch:      noarch
+BuildRequires:  python-lxml
 BuildRequires:  python-devel
 
 %description
@@ -23,6 +24,7 @@ Summary:        Python module to query the lang-table-data
 Group:          Development/Tools
 Requires:       %{name} = %{version}-%{release}
 Requires:       %{name}-data = %{version}-%{release}
+Requires:       python-lxml
 
 %description python
 This package contains a Python module to query the data
@@ -47,7 +49,7 @@ perl -pi -e "s%datadir = '(.*)'%datadir = '%{_datadir}/lang-table'%" langtable.p
 %{__python} setup.py install --skip-build --prefix=%{_prefix} --install-data=%{_datadir}/lang-table --root $RPM_BUILD_ROOT
 
 %check
-#(cd $RPM_BUILD_DIR/%{name}-%{version}; %{__python} -m doctest test_cases.txt)
+(cd $RPM_BUILD_DIR/%{name}-%{version}; %{__python} -m doctest test_cases.txt)
 
 %files
 %doc README COPYING ChangeLog unicode-license.txt test_cases.txt

Comment 2 Parag AN(पराग) 2013-05-08 10:35:58 UTC
+ koji scratch build -> http://koji.fedoraproject.org/koji/taskinfo?taskID=5346037

+ rpmlint on rpms gave
lang-table-python.noarch: W: no-documentation
lang-table-data.noarch: W: summary-not-capitalized C data files for lang-table
lang-table-data.noarch: W: no-documentation
4 packages and 0 specfiles checked; 0 errors, 3 warnings.

==> Fix summary for lang-table-data

Issues:
=======
1) Package contains BR: python2-devel or python3-devel
  See: http://fedoraproject.org/wiki/Packaging:Python#BuildRequires

2) No Requires: %{name}%{?_isa} = %{version}-%{release} in lang-table-python , lang-table-data

3) No need to write for python packages now following in spec for only Fedora specific packages

%{!?python_sitelib: %define python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print get_python_lib()")}


4) One more suggestion for upstream, I am confused actually with the usage of lang-table and langtable words. Can upstream use any single word everywhere?

like package name is lang-table but python source file is langtable.py

Shouldn't then your package name be python-langtable for your python subpackage.
See http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28python_modules.29

5) I am a bit confused about licensing here. all *.py files have header for LGPLv3+ whereas setup.py says that module is in GPLv3+

Also, you need to specify "and MIT" in license tag and add comment about which code is in which license.

I see unicode-license,txt refers to "Modern Style without sublicense (Unicode)" license whose short form is MIT

Comment 3 Mike FABIAN 2013-05-08 14:01:55 UTC
(In reply to comment #2)
> + koji scratch build ->
> http://koji.fedoraproject.org/koji/taskinfo?taskID=5346037
> 
> + rpmlint on rpms gave
> lang-table-python.noarch: W: no-documentation
> lang-table-data.noarch: W: summary-not-capitalized C data files for
> lang-table
> lang-table-data.noarch: W: no-documentation
> 4 packages and 0 specfiles checked; 0 errors, 3 warnings.
> 
> ==> Fix summary for lang-table-data

Done.

> Issues:
> =======
> 1) Package contains BR: python2-devel or python3-devel
>   See: http://fedoraproject.org/wiki/Packaging:Python#BuildRequires

Fixed.

> 2) No Requires: %{name}%{?_isa} = %{version}-%{release} in lang-table-python
> , lang-table-data

Not needed because the package is “noarch” and %{_isa} is needed
only for architecture dependend packages.

> 3) No need to write for python packages now following in spec for only
> Fedora specific packages
> 
> %{!?python_sitelib: %define python_sitelib %(%{__python} -c "from
> distutils.sysconfig import get_python_lib; print get_python_lib()")}

Removed.

> 4) One more suggestion for upstream, I am confused actually with the usage
> of lang-table and langtable words. Can upstream use any single word
> everywhere?
> 
> like package name is lang-table but python source file is langtable.py

Renamed everything to just “langtable”.

> Shouldn't then your package name be python-langtable for your python
> subpackage.
> See
> http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.
> 28python_modules.29

Maybe langtable-python is OK as well, there are many other
<something>-python packages.

> 5) I am a bit confused about licensing here. all *.py files have header for
> LGPLv3+ whereas setup.py says that module is in GPLv3+
> 
> Also, you need to specify "and MIT" in license tag and add comment about
> which code is in which license.
> 
> I see unicode-license,txt refers to "Modern Style without sublicense
> (Unicode)" license whose short form is MIT

Fixed and clarified in the .spec file and README.

New URLs coming in the next comment ...

Comment 5 Parag AN(पराग) 2013-05-08 14:35:55 UTC
Thank you for updating upstream as well as fedora srpm.

APPROVED this package.

Comment 6 Mike FABIAN 2013-05-08 14:48:24 UTC
New Package SCM Request
=======================
Package Name: langtable
Short Description: For guessing reasonable defaults for locale, keyboard, territory, …
Owners: mfabian
Branches: f19
InitialCC: i18n-team

Comment 7 Gwyn Ciesla 2013-05-08 15:01:28 UTC
Requested package name langtable doesn't match bug summary lang-table,
please correct.

Comment 8 Mike FABIAN 2013-05-08 15:08:54 UTC
Fixed bug summary.

Comment 9 Gwyn Ciesla 2013-05-08 16:00:29 UTC
Git done (by process-git-requests).

Comment 10 Parag AN(पराग) 2013-07-15 04:43:42 UTC
This can be closed as package is already in requested branches.

Comment 11 Parag AN(पराग) 2013-07-15 05:07:15 UTC
Christopher,
   I really think its first package submitter or reviewer's call to close this ticket. Only when both are not responding some other contributor can work on this. Sometimes package submitter have their own views of closing the ticket like waiting for another release, pushing via bodhi and then closing the review. The only reason I have not closed this myself is, we should listen to package submitter.

Comment 12 Mike FABIAN 2013-07-15 08:25:12 UTC
It is fine, I think this can be closed.


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