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
Type: ---
Embargoed:


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.