Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: rubygem-bluecloth - A Ruby implementation of John Gruber's Markdown
https://bugzilla.redhat.com/show_bug.cgi?id=771297
Summary: Review Request: rubygem-bluecloth - A Ruby implementation of John Gruber's Markdown Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: mfojtik@redhat.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: --- Regression: --- Mount Type: --- Documentation: ---
Spec URL: http://omicron.mifo.sk/rubygem-bluecloth.spec SRPM URL: http://omicron.mifo.sk/rubygem-bluecloth-2.2.0-1.fc14.src.rpm Description:
BlueCloth is a complete rewrite using David Parsons Discount library, a C implementation of Markdown.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=771297
--- Comment #1 from Michal Fojtik mfojtik@redhat.com 2012-01-03 05:22:26 EST --- Revision -2:
Spec URL: http://omicron.mifo.sk/rubygem-bluecloth.spec SRPM URL: http://omicron.mifo.sk/rubygem-bluecloth-2.2.0-2.fc14.src.rpm
Changelog:
- Fixed wrong binary location
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=771297
Michal Fojtik mfojtik@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |771318
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=771297
--- Comment #2 from Michal Fojtik mfojtik@redhat.com 2012-01-03 11:16:45 EST --- Revision -3:
Spec URL: http://omicron.mifo.sk/rubygem-bluecloth.spec SRPM URL: http://omicron.mifo.sk/rubygem-bluecloth-2.2.0-3.fc14.src.rpm
Koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3615839
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=771297
Vít Ondruch vondruch@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |vondruch@redhat.com
--- Comment #3 from Vít Ondruch vondruch@redhat.com 2012-05-11 04:19:10 EDT --- Michal, could you please update the spec file for latest Fedora? Thank you.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=771297
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(mfojtik@redhat.co | |m)
https://bugzilla.redhat.com/show_bug.cgi?id=771297
Michal Fojtik mfojtik@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(mfojtik@redhat.co | |m) |
--- Comment #4 from Michal Fojtik mfojtik@redhat.com --- Wow, this has been a loong time.. Going to update the spec file now.
https://bugzilla.redhat.com/show_bug.cgi?id=771297
--- Comment #5 from Michal Fojtik mfojtik@redhat.com --- Updated spec file:
Spec URL: http://omicron.mifo.sk/rubygem-bluecloth.spec SRPM URL: http://omicron.mifo.sk/rubygem-bluecloth-2.2.0-3.fc19.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=771297
Vít Ondruch vondruch@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |vondruch@redhat.com
--- Comment #6 from Vít Ondruch vondruch@redhat.com --- I'll take this for a review.
https://bugzilla.redhat.com/show_bug.cgi?id=771297
Vít Ondruch vondruch@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Flags| |fedora-review?
--- Comment #7 from Vít Ondruch vondruch@redhat.com --- * Remove BuildRoot - BuildRoot is not needed, unles you plan to ship this in EPE5 (but in that case, you miss a whole lot of stuff there ;)
* Remove %clean section - Not needed anymore.
* Use the library from %{buildroot}%{gem_instdir}/lib/ - I.e. you should replace:
- mv %{buildroot}%{gem_instdir}/ext/bluecloth_ext.so \ %{buildroot}%{gem_extdir_mri}/ext/%{gem_name}/ + mv %{buildroot}%{gem_libdir}/bluecloth_ext.so \ %{buildroot}%{gem_extdir_mri}/ext/%{gem_name}/
* man pages - Do not compress man pages. That should be done automatically by build system - Refer them as "%doc %{_mandir}/man1/*" in %files section should be enough.
* Directory ownership - %{gem_instdir} is not owned. Please uncomment the "%dir %{gem_instdir}" macro.
* Mark documentation by %doc macro - Documentation should be marked by %doc macro. I am referring to the following files:
%{gem_instdir}/LICENSE %{gem_instdir}/LICENSE.discount %{gem_instdir}/README.rdoc %{gem_instdir}/History.rdoc %{gem_instdir}/Manifest.txt %{gem_instdir}/bluecloth.1.pod
* Do not BR: rubygem(hoe) - Not sure why are you requiring it.
* Test suite - It is definitely not RSpec 1.x only. It runs quite OK with RSpec 2.x. There fails only several specs from spec/markdowntest_spec.rb, due to missing tidy-ext. Please omit just the failing tests.
* License - Not sure about the licenses though. This is the licensecheck output:
$ licensecheck LICENSE LICENSE: BSD (2 clause)
$ licensecheck LICENSE.discount LICENSE.discount: MIT/X11 (BSD like)
- However, the both looks more like BSD then MIT. Could you please check with Fedora Legal?
https://bugzilla.redhat.com/show_bug.cgi?id=771297
Michal Fojtik mfojtik@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC|package-review@lists.fedora | |project.org |
--- Comment #8 from Michal Fojtik mfojtik@redhat.com --- (In reply to Vít Ondruch from comment #7)
- Remove BuildRoot
- BuildRoot is not needed, unles you plan to ship this in EPE5 (but in that case, you miss a whole lot of stuff there ;)
check.
- Remove %clean section
- Not needed anymore.
check.
Use the library from %{buildroot}%{gem_instdir}/lib/
I.e. you should replace:
- mv %{buildroot}%{gem_instdir}/ext/bluecloth_ext.so \ %{buildroot}%{gem_extdir_mri}/ext/%{gem_name}/
- mv %{buildroot}%{gem_libdir}/bluecloth_ext.so \ %{buildroot}%{gem_extdir_mri}/ext/%{gem_name}/
check.
- man pages
- Do not compress man pages. That should be done automatically by build
check.
system
- Refer them as "%doc %{_mandir}/man1/*" in %files section should be enough.
check.
- Directory ownership
- %{gem_instdir} is not owned. Please uncomment the "%dir %{gem_instdir}" macro.
check.
Mark documentation by %doc macro
Documentation should be marked by %doc macro. I am referring to the following files:
%{gem_instdir}/LICENSE %{gem_instdir}/LICENSE.discount %{gem_instdir}/README.rdoc %{gem_instdir}/History.rdoc %{gem_instdir}/Manifest.txt %{gem_instdir}/bluecloth.1.pod
check.
- Do not BR: rubygem(hoe)
- Not sure why are you requiring it.
check.
- Test suite
- It is definitely not RSpec 1.x only. It runs quite OK with RSpec 2.x. There fails only several specs from spec/markdowntest_spec.rb, due to missing tidy-ext. Please omit just the failing tests.
done.
License
Not sure about the licenses though. This is the licensecheck output:
$ licensecheck LICENSE LICENSE: BSD (2 clause)
$ licensecheck LICENSE.discount LICENSE.discount: MIT/X11 (BSD like)
The 'discount' license is for the 'discount' ruby gem which I guess this gem was bundling or something. However we are not bundling anything, so I think BSD is correct here.
Thanks Vit for this review!
Spec URL: http://omicron.mifo.sk/rubygem-bluecloth.spec SRPM URL: http://omicron.mifo.sk/rubygem-bluecloth-2.2.0-4.fc19.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=771297
--- Comment #10 from Vít Ondruch vondruch@redhat.com --- Ping ... Any progress?
https://bugzilla.redhat.com/show_bug.cgi?id=771297
Vít Ondruch vondruch@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Blocks| |201449 (FE-DEADREVIEW) Resolution|--- |NOTABUG Flags|fedora-review? | Last Closed| |2016-05-13 05:29:24
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=201449 [Bug 201449] FE-DEADREVIEW -- Reviews stalled due to lack of submitter response should be blocking this bug.
package-review@lists.fedoraproject.org