https://bugzilla.redhat.com/show_bug.cgi?id=1343977 Vít Ondruch <vondruch@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(vondruch@redhat.c | |om) | --- Comment #11 from Vít Ondruch <vondruch@xxxxxxxxxx> --- (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@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx