Bug 434954
Summary: | Review Request: WritRecogn - A CJK handwriting recognizer | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Ding-Yi Chen <dchen> | ||||||||
Component: | Package Review | Assignee: | Jens Petersen <petersen> | ||||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||||
Severity: | medium | Docs Contact: | |||||||||
Priority: | medium | ||||||||||
Version: | rawhide | CC: | eng-i18n-bugs, fedora-package-review, mtasaka, notting, panemade, petersen, peter | ||||||||
Target Milestone: | --- | Flags: | petersen:
fedora-review+
petersen: fedora-cvs+ |
||||||||
Target Release: | --- | ||||||||||
Hardware: | All | ||||||||||
OS: | Linux | ||||||||||
Whiteboard: | |||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||
Doc Text: | Story Points: | --- | |||||||||
Clone Of: | Environment: | ||||||||||
Last Closed: | 2008-03-03 01:22:21 UTC | Type: | --- | ||||||||
Regression: | --- | Mount Type: | --- | ||||||||
Documentation: | --- | CRM: | |||||||||
Verified Versions: | Category: | --- | |||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||
Embargoed: | |||||||||||
Attachments: |
|
Description
Ding-Yi Chen
2008-02-26 14:52:51 UTC
rpmlint says: writRecogn.src: W: summary-ended-with-dot A handwriting recognizer that allows users to input Chinese characters used in Chinese, Japanese and Korean. writRecogn.src: E: summary-too-long A handwriting recognizer that allows users to input Chinese characters used in Chinese, Japanese and Korean. writRecogn.src: E: description-line-too-long writRecogn recognizes east Asian handwritten characters for Chinese, Japanese, and Korean (CJK) and will interface to input methods such as SCIM. Unlike some implementations which require to build a huge set of character recognition rules, we recognize radicals of Chinese characters, i.e. the word root of the character, then use a character-structure-based input method to search for the word. This saves us from writing recognition rules for tens of thousands of CJK characters. This should provide better recognition accuracy than current open source handwriting recognition libraries, like tomoe. writRecogn.src: E: description-line-too-long The main program which provides the GUI is writRecogn and there is a commandline character data maintenance program cdMgr. Personally I would suggest calling it WritRecogn or writrecogn. The mixed lower- and uppercase looks a little awkward to me, for the record anyway. | BuildRequires: gtk2-devel >= 2.10 libxml2-devel >= 2.6 sqlite-devel >= 3.0 libtool >= 1.5 | Requires: gtk2 >= 2.10 libxml2 >= 2.6 sqlite >= 3.0 I think it should sufficient to specific the minimum versions for the buildrequires and then the explicit Requires can be droppped. They are all satisfied by Fedora 7 anyway so not really necessary for Fedora anyway. Program seems to run and work ok - I realise it is still alpha at this stage. I just note a warning at runtime though: ** (writRecogn:9633): WARNING **: Couldn't find pixmap file: lightbulb.png and also some debug output (but that is ok at this stage). Also just wondering if it is necessary to ship all the files in "/usr/share/writRecogn/data/"? Again about naming I would suggest naming the programs something like %{_bindir}/WritRecogn and %{_bindir}/WritRecogn-manager. It would be nice to have a .desktop file for the main program. Created attachment 296018 [details]
writRecogn.spec-1.patch
minor spec cleanup suggestions
Jens, Thanks for your comments. Regarding comment #2: The building requires are mainly for koji build, otherwise the koji cannot find essential libraries. Regarding comment #3: The lightbulb.png shows a small icon in "Recognize" buttom. This will be fixed in next release. Regarding comment #4,#5: Addressed accordingly. Regards, Ding-Yi Chen Okay thanks. Please update the URLs for each revision. :) Spec URL: http://downloads.sourceforge.net/writrecogn/writRecogn.spec SRPM URL: http://downloads.sourceforge.net/writrecogn/WritRecogn-0.1.1-1.fc8.src.rpm rpmlint still says: WritRecogn.src: E: description-line-too-long writRecogn recognizes East Asian handwritten characters for Chinese, Japanese, and Korean (CJK). Unlike some implementations which require to build a huge set of character recognition rules, we recognize radicals of Chinese characters, i.e. the word root of the character, then use a character-structure-based input method to search for the word. This saves us from writing recognition rules for tens of thousands of CJK characters. This should provide better recognition accuracy than current open source handwriting recognition libraries. WritRecogn.src: E: description-line-too-long The main program which provides the GUI is writRecogn and there is a commandline character data maintenance program cdMgr. > Regarding comment #3: > The lightbulb.png shows a small icon in "Recognize" buttom. This will be fixed > in next release. In version 0.2? (In reply to comment #3) > Also just wondering if it is necessary to ship all the files in > "/usr/share/writRecogn/data/"? How about this? (In reply to comment #4) > It would be nice to have a .desktop file for the main program. See http://fedoraproject.org/wiki/Packaging/Guidelines#head-254ddf07aae20a23ced8cecc219d8f73926e9755 for details about .desktop files. I think you need to give credit for the zh handwriting data from tomoe (which is under LGPL2+). Created attachment 296143 [details]
WritRecogn.spec-2.patch
Patch to fix rpmlint warnings.
As for /usr/share/writRecogn/data: Files starting with handwriting are required, others are just examples. Tomoe does not have .desktop, but cellwrite does. :-) Will add the .desktop on 0.2 for main program. WritRecogn-manager, on the other hand, is a command line program, thus the .desktop is not required,. An acknowledge section is appended to AUTHORS. New spec: http://dchen.fedorapeople.org/files/rpms/WritRecogn.spec New srpm: http://dchen.fedorapeople.org/files/rpms/ WritRecogn-0.1.1-2.fc8.src.rpm Regards, Thanks for the update. (In reply to comment #12) > As for /usr/share/writRecogn/data: Files starting with handwriting are > required, others are just examples. Ok, then let's remove the examples from the binary package? > Tomoe does not have .desktop, but cellwrite does. :-) scim-tomoe requires scim and has a menu entry on the SCIM command menu so it is ok. Until WritRecogn is integrated with Input Methods it needs to have a desktop menu. > An acknowledge section is appended to AUTHORS. Thanks Here is the review: +: ok, -: needs attention MUST Items: [+] MUST: rpmlint must be run on every package. [+] MUST: The package must be named according to the Package Naming Guidelines. [+] MUST: The spec file name must match the base package %{name} [+] MUST: The package must meet the Packaging Guidelines. Better to set BuildRoot to either: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) or %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) [+] MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. [+] MUST: The License field in the package spec file must match the actual license. [+] MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc. [+] MUST: The spec file must be written in American English. [+] MUST: The spec file for the package MUST be legible. [-] MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this. They do not match. Please update the upstream source file (or better bump the upstream version). [+] MUST: The package must successfully compile and build into binary rpms on at least one supported architecture. [+] MUST: All build dependencies must be listed in BuildRequires [-] MUST: A package must own all directories that it creates. Need to own %{_datadir}/%{name} [+] MUST: A package must not contain any duplicate files in the %files listing. [+] MUST: Permissions on files must be set properly. [+] MUST: Each package must have a %clean section [+] MUST: Each package must consistently use macros [+] MUST: The package must contain code, or permissable content. [+] MUST: If a package includes something as %doc, it must not affect the runtime of the application. [-] MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. This is described in detail in the desktop files section of Packaging Guidelines. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation. [+] MUST: Packages must not own files or directories already owned by other packages. [+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). See Prepping BuildRoot For %install for details. [+] MUST: All filenames in rpm packages must be valid UTF-8. SHOULD Items: [+] SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available. [+] SHOULD: The reviewer should test that the package builds in mock. [+] SHOULD: The reviewer should test that the package functions as described. Hi, Comment #14 addressed. New spec: http://dchen.fedorapeople.org/files/rpms/WritRecogn.spec New srpm: http://dchen.fedorapeople.org/files/rpms/ WritRecogn-0.1.1-3.fc8.src.rpm In future please describe the actual changes in the .spec changelog. I think you need the full path to the icon in the desktop file for it to display. You need to use a tarball from upstream in your package. Otherwise it looks ok to me now. Created attachment 296168 [details]
WritRecogn.spec-3.patch
few more suggestions
Some comments: - When installing icons under %_datadir/icons/hicolor/, icon cache must be updated. Otherwide gnome panel entry won't display the installed icon correctly ("GTK+ icon cache" of http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ) - build.log shows (for example: http://koji.fedoraproject.org/koji/getfile?taskID=474798&name=build.log ) ------------------------------------------------------------------------ config.status: creating src/utils/Makefile config.status: creating data/Makefile config.status: creating config.h config.status: executing depfiles commands + make -j2 cd . && /bin/sh /builddir/build/BUILD/WritRecogn-0.1.1/missing --run aclocal-1.10 cd . && /bin/sh /builddir/build/BUILD/WritRecogn-0.1.1/missing --run autoconf cd . && /bin/sh /builddir/build/BUILD/WritRecogn-0.1.1/missing --run automake-1.10 --foreign /bin/sh ./config.status --recheck running CONFIG_SHELL=/bin/sh /bin/sh ./configure --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu --target=x86_64-redhat-linux-gnu --program-prefix= --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/usr/com --mandir=/usr/share/man --infodir=/usr/share/info --disable-static build_alias=x86_64-redhat-linux-gnu host_alias=x86_64-redhat-linux-gnu target_alias=x86_64-redhat-linux-gnu CFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic CXXFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic FFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic --no-create --no-recursion --------------------------------------------------------------------- Here autotools are called after configure is finished and then configure is rechecked. This means timestamps of some files must be fixed (i.e. some files need "touch"ing). As shown in http://fedoraproject.org/wiki/Packaging/Guidelines#head-254ddf07aae20a23ced8cecc219d8f73926e9755 The short form is prefered. The icon cache issue has been dealt with in 0.1.1-4 New spec:http://dchen.fedorapeople.org/files/rpms/WritRecogn.spec New srpm: http://dchen.fedorapeople.org/files/rpms/WritRecogn-0.1.1-4.fc8.src.rpm Thanks, I think the package now satisfies all the Packaging Requirements. (As Tasaka commented it would be better to fix the build configure issue.) Package is APPROVED. New Package CVS Request ======================= Package Name: WritRecogn Short Description: A CJK handwriting recognizer Owners: dchen Branches: F-7 F-8 InitialCC: dchen Cvsextras Commits:yes cvs admin done |