https://bugzilla.redhat.com/show_bug.cgi?id=1343977
Vít Ondruch <vondr...@redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags|needinfo?(vondruch@redhat.c |
|om) |
--- Comment #11 from Vít Ondruch <vondr...@redhat.com> ---
(In reply to Tummala Dhanvi (c0mrad3) from comment #10)
> (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`
There are two things here:
1. If you do something special, mainly if the SourceX tag is not link, it is
always good idea to describe in comment what you actually did. Some people even
prefer to provide script preparing the packages (isn't this documented
somewhere in guidelines? Not sure)
2. More importantly, I don't think this is required and that this is right
approach here. I would suggest to get the tarball from the GH [1], expand it
using %setup macro and that should be everything you have in %prep section. The
rest of the package would be the same.
As soon as there is stable version of the gem available on rubygems.org, then
you would change the package to use the .gem as a source.
> > * 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).
> > * 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"
```
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).
[1] https://fedoraproject.org/wiki/Packaging:SourceURL#Commit_Revision
[2] https://fedoraproject.org/wiki/Packaging:Versioning#Pre-Release_packages
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
