Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 771297 - Review Request: rubygem-bluecloth - A Ruby implementation of John Gruber's Markdown
Summary: Review Request: rubygem-bluecloth - A Ruby implementation of John Gruber's Ma...
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Vít Ondruch
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW 771318
TreeView+ depends on / blocked
 
Reported: 2012-01-03 08:19 UTC by Michal Fojtik
Modified: 2016-05-13 09:29 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-05-13 09:29:24 UTC


Attachments (Terms of Use)

Description Michal Fojtik 2012-01-03 08:19:41 UTC
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.

Comment 1 Michal Fojtik 2012-01-03 10:22:26 UTC
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

Comment 3 Vít Ondruch 2012-05-11 08:19:10 UTC
Michal, could you please update the spec file for latest Fedora? Thank you.

Comment 4 Michal Fojtik 2013-09-25 09:25:42 UTC
Wow, this has been a loong time.. Going to update the spec file now.

Comment 5 Michal Fojtik 2013-09-25 11:59:16 UTC
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

Comment 6 Vít Ondruch 2013-09-26 10:52:10 UTC
I'll take this for a review.

Comment 7 Vít Ondruch 2013-09-26 11:46:15 UTC
* 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?

Comment 8 Michal Fojtik 2013-09-26 14:56:09 UTC
(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

Comment 9 Vít Ondruch 2013-09-27 08:17:02 UTC
(In reply to Michal Fojtik from comment #8)
> > * 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.

Wrong and wrong.

1) There is discount library: http://www.pell.portland.or.us/~orc/Code/discount/
2) It is actually bundling. I would consider it for personally, but others may disagree. Better to ask FPC as well as legal for the license.

And since you mentioned rubygem-rdiscount and bundling:

https://bugzilla.redhat.com/show_bug.cgi?id=964940

I wonder, what is difference between libmarkdown and discount.

Comment 10 Vít Ondruch 2015-09-15 12:31:59 UTC
Ping ... Any progress?


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