[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 #12 from Tummala Dhanvi (c0mrad3) <dhanvicse@xxxxxxxxx> ---
> 
> 
> > > * 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
> 
> Please read the [2] carefully. You have to ensure update path. E.g. the
> original VR you used was "0.1.0.dev-1". If the upstream released stable
> version "0.1.0", your VR would become "0.1.0.dev-1" and this is what RPM
> thinks about the versions:
> 
> ```
> $ rpmdev-vercmp 0.1.0.dev-1 0.1.0-1
> 0.1.0.dev-1 > 0.1.0-1
> ```
> 
> That means the newer version would never get installed. Hence I suggested to
> use just "0.1.0" while the release should be "0.1.dev". In the .spec file,
> it would look like:
> 
> ```
> Version: 0.1.0
> Release: 0.1.dev%{?dist}
> ```
> 
> Which makes the upgrade path correct:
> 
> ```
> $ rpmdev-vercmp 0.1.0-0.1.dev 0.1.0-1
> 0.1.0-0.1.dev < 0.1.0-1
> ```
> 
> Again, please read the [2] carefully (although I admit it is not easy read,
> but hopefully this will get simplified in the future).
Got it right this time!
> 
> > > * 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/
> 
> My idea was to do something as simple as shipping some simple AsciiDoc file
> as Source1 and trying the conversion, e.g. the %check section could look
> like:
> 
> ```
> %check
> pushd .%{gem_instdir}
> # There is no rubygem-asciidoctor-doctest in Fedora yet, which is needed
> # for test suite.
> # ruby -Ilib -e 'Dir.glob "./test/*_test.rb", &method(:require)'
> 
> bin/asciidoctor-mallard %{SOURCE1} -o test.output
> popd
> 
> ```
> 
> This is the simplest test you can do, since if the conversion fails for
> whatever reason, the build should break.
> 
> On top of that, you can check for the consistency of the output by
> calculating hash of the output, e.g. you can add the following line there:
> 
> ```
> sha256sum test.output | grep
> "870474333c6f4e41238f923f1d27c7688c86b6463db39725e076fc5f05fe2520" 
> ```
done it using the example given in the README
> 
> Of course you have to use correct hash, based on the content of the
> %{SOURCE1} file you are going to use. Or you can choose to test any other
> bit of the file content, if you consider it important.
> 
> 
> BTW just running "bin/asciidoctor-mallard" will fail the build ATM, since
> there is apparently some bug (undefined method `size' for nil:NilClass).
> 
This might be due to the requirements of rubygem-asciidoctor which I missed,
also 

`
ruby ./bin/asciidoctor-mallard
`

works.

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]