Spec URL: http://skottler.fedorapeople.org/reviews/rubygem-rgen/rubygem-rgen.spec SRPM URL: http://skottler.fedorapeople.org/reviews/rubygem-rgen/rubygem-rgen-0.6.6-1.fc21.src.rpm Description: RGen is a framework for Model Driven Software Development (MDSD) in Ruby. This means that it helps you build Metamodels, instantiate Models, modify and transform Models and finally generate arbitrary textual content from it. Fedora Account System Username: skottler
I'll take this for a review.
* %clean may not be required - %clean is required just for EPEL5 [1], are you planning to submit this package for EPEL5? Make this section optional just for EPEL5 might be good idea as well. Not a blocker though. * License file in main package - The license file have to be shipped in the main package [2]. * Wrong conditionalized ruby requires - "%if 0%{?rhel} <= 6" is on Fedora always true, since %{rhel} macro is not defined. This results in "Error: No Package found for ruby(abi) = 1.8" during build. * BuildRequires: ruby - There is no need for "BuildRequires: ruby" as long as the gem is noarch. "BuildRequires: ruby(release)" should be enough. * Move test and Rakefile into -doc subpackage - Files which are not needed for runtime are typically placed into -doc subpackage. [1] https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean [2] https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text
(In reply to Vít Ondruch from comment #2) > * %clean may not be required > - %clean is required just for EPEL5 [1], are you planning to submit this > package for EPEL5? Make this section optional just for EPEL5 might be > good > idea as well. Not a blocker though. Nope, definitely not going to put the package in EL5. Removed. > > * License file in main package > - The license file have to be shipped in the main package [2]. Moved it. > > * Wrong conditionalized ruby requires > - "%if 0%{?rhel} <= 6" is on Fedora always true, since %{rhel} macro is not > defined. This results in "Error: No Package found for ruby(abi) = 1.8" > during build. I removed the conditional completely - when I pull the package into EPEL6 I'll just change the package names. > > * BuildRequires: ruby > - There is no need for "BuildRequires: ruby" as long as the gem is noarch. > "BuildRequires: ruby(release)" should be enough. Removed. > > * Move test and Rakefile into -doc subpackage > - Files which are not needed for runtime are typically placed into -doc > subpackage. Moved them into the -doc subpackage. > > > > [1] https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean > [2] https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text
The spec and srpm are updated at the same link.
* Bump release - It is good habit to bump release for each review cycle. It is easier to see changes and compare with previous version, if needed. * Don't mark test and Rakefile by %doc macro - They are not documentation, therefore they should not be marked by %doc documentation. - Some even thinks that nothing in -doc subpackage should be marked by as %doc, but I am not in that camp ;) So take it just as a remark. * Remove test and Rakefile from main package - You keep them twice now. Please remove them from the main package and leave them just in -doc subpackage * rpmlint - rpmlint complains about wrong-file-end-of-line-encoding a lot. This might be better to check with upstream. - rubygem-rgen-doc.noarch: W: file-not-utf8 /usr/share/gems/gems/rgen- 0.6.6/test/testmodel/ea_testmodel.xml error seems to be false positive, since the XML explicitly says it is windows-1252 encoded. Probably nothing we can do about it. * Test suite - Please execute the test suite during build.
(In reply to Vít Ondruch from comment #5) > * Bump release > - It is good habit to bump release for each review cycle. It is easier to > see I've done that this time: http://skottler.fedorapeople.org/reviews/rubygem-rgen/rubygem-rgen-0.6.6-2.fc21.src.rpm http://skottler.fedorapeople.org/reviews/rubygem-rgen/rubygem-rgen.spec > changes and compare with previous version, if needed. > > * Don't mark test and Rakefile by %doc macro > - They are not documentation, therefore they should not be marked by %doc > documentation. > - Some even thinks that nothing in -doc subpackage should be marked by as > %doc, but I am not in that camp ;) So take it just as a remark. Done. > > * Remove test and Rakefile from main package > - You keep them twice now. Please remove them from the main package and > leave > them just in -doc subpackage Whoops, fixed. > > * rpmlint > - rpmlint complains about wrong-file-end-of-line-encoding a lot. This > might be > better to check with upstream. > - rubygem-rgen-doc.noarch: W: file-not-utf8 /usr/share/gems/gems/rgen- > 0.6.6/test/testmodel/ea_testmodel.xml error seems to be false positive, > since the XML explicitly says it is windows-1252 encoded. Probably > nothing > we can do about it. I'll start working with the upstream to fix this. > > * Test suite > - Please execute the test suite during build. Done.
* Don't introduce additional requires - Upstream does not like them - They are useless, probably except tests with Ruby 1.8.7 * Mark the license by %doc macro - Please mark the %{gem_instdir}/MIT-LICENSE as a documentation. > > * rpmlint > > - rpmlint complains about wrong-file-end-of-line-encoding a lot. This > > might be > > better to check with upstream. > > - rubygem-rgen-doc.noarch: W: file-not-utf8 /usr/share/gems/gems/rgen- > > 0.6.6/test/testmodel/ea_testmodel.xml error seems to be false positive, > > since the XML explicitly says it is windows-1252 encoded. Probably > > nothing > > we can do about it. > > I'll start working with the upstream to fix this. Interesting, now I cannot reproduce these issues. Rpmlint now complains just about CHANGELOG. This is probably updated file utility. Nevertheless, is there any upstream ticket for this? Otherwise, the package looks ok => APPROVED. Please fix the minor nits I have mentioned above prior importing.
(In reply to Vít Ondruch from comment #7) > * Don't introduce additional requires > - Upstream does not like them > - They are useless, probably except tests with Ruby 1.8.7 I just changed it to use RUBYOPTS to load rubygems before running the test suite. > > * Mark the license by %doc macro > - Please mark the %{gem_instdir}/MIT-LICENSE as a documentation. > > > > * rpmlint > > > - rpmlint complains about wrong-file-end-of-line-encoding a lot. This > > > might be > > > better to check with upstream. > > > - rubygem-rgen-doc.noarch: W: file-not-utf8 /usr/share/gems/gems/rgen- > > > 0.6.6/test/testmodel/ea_testmodel.xml error seems to be false positive, > > > since the XML explicitly says it is windows-1252 encoded. Probably > > > nothing > > > we can do about it. > > > > I'll start working with the upstream to fix this. > > Interesting, now I cannot reproduce these issues. Rpmlint now complains just > about CHANGELOG. This is probably updated file utility. > > Nevertheless, is there any upstream ticket for this? Not yet, I'll create one today. I haven't actually tried to reproduce it, but will do that again to see if I can recreate what you've seen before filing the ticket. > > Otherwise, the package looks ok => APPROVED. Please fix the minor nits I > have mentioned above prior importing. Thanks, Vit!
New Package SCM Request ======================= Package Name: rubygem-rgen Short Description: Ruby Modelling and Generator Framework Owners: skottler Branches: f20 f19 el6 epel7 InitialCC:
Git done (by process-git-requests).
rubygem-rgen-0.6.6-2.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/rubygem-rgen-0.6.6-2.fc20
rubygem-rgen-0.6.6-2.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/rubygem-rgen-0.6.6-2.fc19
rubygem-rgen-0.6.6-2.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/rubygem-rgen-0.6.6-2.el6
rubygem-rgen-0.6.6-2.el6 has been pushed to the Fedora EPEL 6 testing repository.
rubygem-rgen-0.6.6-2.fc20 has been pushed to the Fedora 20 stable repository.
rubygem-rgen-0.6.6-2.fc19 has been pushed to the Fedora 19 stable repository.
rubygem-rgen-0.6.6-2.el6 has been pushed to the Fedora EPEL 6 stable repository.