[Bug 1343977] Review Request: rubygem-asciidoctor-mallard - A Project Mallard converter for AsciiDoc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Tummala Dhanvi (c0mrad3) <dhanvicse@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(vondruch@redhat.c
                   |                            |om)



--- Comment #10 from Tummala Dhanvi (c0mrad3) <dhanvicse@xxxxxxxxx> ---
(In reply to Vít Ondruch from comment #9)
> * Source of the gem?
>   - Where did you get the source gem? It is not available on rubygems.org as
> far
>     as I can tell, neither you documented anywhere how you obtained it.
The spec file contained the url of the github repo
https://github.com/asciidoctor/asciidoctor-mallard

I have built the gem my self using `gem build *.gemspec`
> 
> * Package naming
>   - Please rename the .spec file to rubygem-asciidoctor-mallar.spec to be in
>     line with the guidelines [1].
Updated
> 
> * Package version
>   - Please use proper version scheme. The package version should be probably
>     just "0.1.0" while the release should be "0.1.dev".
>   - The versioning is documented in detail here [2].
I upstream had it versioning like that
https://github.com/asciidoctor/asciidoctor-mallard/blob/master/lib/asciidoctor-mallard/version.rb

but I have updated it to 0.1.dev
> 
> * Duplicated license
>   - You should not include the "%license LICENSE.adoc". Please keep just
> "%doc
>     %{gem_instdir}/LICENSE.adoc" in the main package (and change the %doc to
>     %license of course).

Done

> * Unneeded requires
>   - You probably don't need to include "Requires: rubygem-thread_safe".
> Runtime
>     requires should be autogenerated during build process.

Done

>   - Not sure what is the "pintail" good for. Some explaining hint above would
>     come handy.
pintail is actually not required

Pintail is a tool to convert mallard to html
(https://github.com/projectmallard/pintail)

This package converts asciidoc to mallard  
> 
> * Doc subpackage
>   - It is good habit to keep the documentation in -doc subpackage.
Done
> 
> * Test suite
>   - Well, the line you used is just part of the story. You can see that there
>     is nothing which would indicated, that the test suite was executed. You
>     should use following line:
> 
> ```
> ruby -Ilib -e 'Dir.glob "./test/*_test.rb", &method(:require)'
> ```
> 
>   - Unfortunately, there seems to be dependency on asciidoctor-doctest, which
>     is not in Fedora yet. Since it has quite lot of dependencies, it is
>     probably not worth of packaging ATM, but all this should be documented.
>   - Instead of execution of test suite, you should consider to provide at
> least
>     some sanity test, e.g. try to convert some document from AsciiDoc to
>     Mallard using the %{gem_instdir}/bin/asciidoctor-mallard
> 
Can you please explain me a bit more about how to get it right ? 

URL: https://dhanvi.fedorapeople.org/packages/asciidoctor-mallard/

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]