Bug 561452 - Review Request: yydebug - Supports tracing and animation for a Java-based parser generated by jay
Summary: Review Request: yydebug - Supports tracing and animation for a Java-based par...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Alexander Kurtakov
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 561484
TreeView+ depends on / blocked
 
Reported: 2010-02-03 17:13 UTC by Mo Morsi
Modified: 2010-05-11 19:41 UTC (History)
3 users (show)

Fixed In Version: yydebug-1.1.0-5.fc12
Clone Of:
Environment:
Last Closed: 2010-05-10 23:46:36 UTC
Type: ---
Embargoed:
akurtako: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)
Patch adding javadoc generation (362 bytes, patch)
2010-05-05 09:08 UTC, Alexander Kurtakov
no flags Details | Diff

Description Mo Morsi 2010-02-03 17:13:30 UTC
Spec URL: http://mo.morsi.org/files/jruby/yydebug.spec
SRPM URL: http://mo.morsi.org/files/jruby/yydebug-1.1.0-1.fc12.src.rpm
Description: 
jay/yydebug supports tracing and animation for a Java-based parser generated 
by jay. An implementation of yyDebug is passed as an additional argument to 
yyparse() to trace a Java-based parser generated by jay with option -t set.
yyDebugAdapter produces one-line messages, by default to standard output. 
The messages are designed to be filtered by a program such as grep. yyAnim 
provides an animation of the parsing process

Required by JRuby.

Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=1959840

rpmlint -i rpmbuild/RPMS/i386/yydebug-1.1.0-1.fc12.i386.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

rpmlint -i rpmbuild/SRPMS/yydebug-1.1.0-1.fc12.src.rpm 
yydebug.src:87: W: libdir-macro-in-noarch-package (main package) 
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

Ignoring this last warning as when package is noarch (eg if using java-openjdk) the libdir file will not be included, and when the package is architecture specific (eg when using gcj) it will be. See spec file for conditionals.

Comment 1 Alexander Kurtakov 2010-04-26 18:50:49 UTC
I'm taking this one.

Comment 2 Alexander Kurtakov 2010-04-26 18:54:36 UTC
Please drop gcj_support.
THere is no javadoc subpackage which should be present.

Comment 3 Jason Tibbitts 2010-04-26 19:00:13 UTC
Why does gcj support need to be dropped when the guidelines indicate that it should always be included if possible?

Comment 4 Mo Morsi 2010-04-27 21:30:37 UTC
Updated, gcj removed, no javadocs provided by upstream so no subpackage is necessary 

New spec / srpm:
Spec URL: http://mo.morsi.org/files/jruby/yydebug.spec
SRPM URL: http://mo.morsi.org/files/jruby/yydebug-1.1.0-2.fc11.src.rpm

See my comment here for more info on the gcj bits:
https://bugzilla.redhat.com/show_bug.cgi?id=561451#c5

Comment 5 Alexander Kurtakov 2010-05-04 16:39:27 UTC
Review:

OK: rpmlint must be run on every package. Output:

yydebug.src: W: spelling-error %description -l en_US yyparse -> parsec, sparse, parse
yydebug.src: W: spelling-error %description -l en_US yyAnim -> cyanide, animus, animal

False positives.

OK: The package must be named according to the Package Naming Guidelines .
OK: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption. 
OK: The package must meet the Packaging Guidelines .
NOTOK: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines . 

Where did you get the License info from. I haven't found anything on the topic on the site and in the sources
 
NOTOK: The License field in the package spec file must match the actual license. 

See prievious.

OK: 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.
OK: The spec file must be written in American English. 
OK: The spec file for the package MUST be legible. 
OK: 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. 
OK: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
OK: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense.
OK: Packages must NOT bundle copies of system libraries.
OK: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. 
OK: A Fedora package must not list a file more than once in the spec file's %files listings. 
OK: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line. 
OK: Each package must consistently use macros. 
OK: The package must contain code, or permissable content. 
OK: Large documentation files must go in a -doc subpackage. 
OK: If a package includes something as %doc, it must not affect the runtime of the application. 
OK: Packages must not own files or directories already owned by other packages. 
OK: All filenames in rpm packages must be valid UTF-8. 


Licensing should be cleared - you have to put info in the spec file where you get the license from.
About javadocs I would understand if you don't want to do it but adding javadoc generation should be just a few lines to the makefile and it's really good to have.

Comment 6 Mo Morsi 2010-05-04 19:42:32 UTC
New Spec/SRPM w/ license source
Spec URL: http://mo.morsi.org/files/jruby/yydebug.spec
SRPM URL: http://mo.morsi.org/files/jruby/yydebug-1.1.0-3.fc11.src.rpm


License is set to BSD since the 'jay' project, which 'yydebug' is a subproject under is licensed under BSD. See the jay.1 man page and the src/ subdir in the project repo:

http://svn.codehaus.org/jruby/trunk/jay/
http://www.cs.rit.edu/~ats/projects/lp/doc/jay/doc-files/src.zip

AFAIK (based on looking at the imports/dependencies) jruby only depends on yydebug and not the jay superproject, thus I only packaged yydebug.

As far as javadocs, I'm more that willing to put them in, I'm not just that familiar w/ the javadoc system. If you want to send me a makefile patch I can include it, update the spec, and regenerate the srpm.

Thanks for the review.

Comment 7 Alexander Kurtakov 2010-05-05 09:08:39 UTC
Created attachment 411522 [details]
Patch adding javadoc generation

Comment 8 Alexander Kurtakov 2010-05-05 09:09:27 UTC
This patch should be applied after your current one.

Comment 10 Alexander Kurtakov 2010-05-06 13:22:46 UTC
%changelog entries are missing your name. Otherwise the package looks good.
Please make sure that you fix the %changelog before committing.

This package is APPROVED.

Comment 11 Mo Morsi 2010-05-06 16:41:18 UTC
Ah yes, my mistake. New version of the spec / srpm, with my name added to the changelog (the only change):

New Spec/SRPM w/ javadocs
Spec URL: http://mo.morsi.org/files/jruby/yydebug.spec
SRPM URL: http://mo.morsi.org/files/jruby/yydebug-1.1.0-5.fc11.src.rpm

New Package CVS Request
=======================
Package Name: yydebug
Short Description: Supports tracing and animation for a Java-based parser generated by jay.
Owners: mmorsi
Branches: F-12 F-13
InitialCC:

Comment 12 Kevin Fenzi 2010-05-09 02:05:56 UTC
CVS done (by process-cvs-requests.py).

Comment 13 Fedora Update System 2010-05-10 22:14:58 UTC
yydebug-1.1.0-5.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/yydebug-1.1.0-5.fc12

Comment 14 Fedora Update System 2010-05-10 22:15:21 UTC
yydebug-1.1.0-5.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/yydebug-1.1.0-5.fc13

Comment 15 Fedora Update System 2010-05-10 23:46:32 UTC
yydebug-1.1.0-5.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 16 Fedora Update System 2010-05-11 19:41:09 UTC
yydebug-1.1.0-5.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.


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