[Bug 1361334] Review Request: rubygem-rails-controller-testing - Extracting `assigns` and `assert_template` from ActionDispatch

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

 



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



--- Comment #7 from Jun Aruga <jaruga@xxxxxxxxxx> ---
(In reply to František Dvořák from comment #5)

> > #1. LICENCE file.
> 
> Pull request sent (and merged). It seems it is just a typo in upstream
> gemspec and it's true it will simplify the spec file.
> 
> URL updated, versioned license file is better.

Okay I got it, and checked your new spec file.
It looks good to me. Thanks.

> > #2. Koji URL
> > 
> > Next time not now, it is good habit to put the result of scratch build.
> > https://bugzilla.redhat.com/show_bug.cgi?id=1359224
> > > Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=14979792
> > 
> 
> OK (I usually include the link to the build. :-))

OK :)

> > #3. files
> > 
> > %files
> > %doc %{gem_instdir}/README.md
> > 
> > You may use gem2rpm old version.
> > You do not need to include README.md %files section. as you inlude LICENSE
> > file in it.
> > 
> > Remove below lines from %files, and move to %files doc section.
> > I want to include below files doc RPM file.
> > 
> > %exclude %{gem_instdir}/test/
> > %exclude %{gem_instdir}/Rakefile
> > 
> > The reason is we want to reduce the base RPM's file size.
> > And if it is not ".*" file, want to include doc RPM file.
> > 
> 
> OK. I guess this is quite "gray" area: some reviewers even prefer to include
> README file in the main package. The gem2spec version way may be slightly
> better here because more packages will probably use that way. :-) Updated.

Yeah, some reviewers may prefer to include README file.
It is grey area, as it is not written on "Packaging:Ruby" Guideline.
You can decide it of cource finally.
OK I checked it had updated in the new spec file.
Thanks.

> > #3. Add below lines. near "BuildRequires: rubygems-devel".
> >   You can see the result of latest gem2rpm.
> > 
> > BuildRequires: ruby(release)
> > BuildRequires: ruby
> > 
> 
> Why is that needed? Guideline doesn't mention it (most of my ruby packages
> doesn't have these dependencies).
> 
> Although it's true I haven't checked any other ruby implementations here.

We can build and install without these 2 lines on latest version Fedora.
Because these 2 lines are needed to build, install on older Feodra version like
F22 or F23. Though I do not know exact version for that. I have heard it.
However it is gray area. It is up to you finally.

> > #5. Use latest version gem2rpm
> > 
> > Finally latest gem2rpm is covering above these things.
> > I can suggest you can use master branch of 
> > https://github.com/fedora-ruby/gem2rpm
> > , though it is optional.
> > 
> > Its generated template is the collection of our latest trend for the spec
> > file.
> > 
> 
> Yes, it looks like there are no differences in generated spec-file between
> the version I use (0.11.3) and the master branch.
> 
> I generate the spec file by gem2spec, but edit it heavily after that.

Yes mostly same. There is a little different. But it looks harmless.
https://github.com/fedora-ruby/gem2rpm/issues/87

> Updated package (koji
> http://koji.fedoraproject.org/koji/taskinfo?taskID=15175903):
> 
> Spec URL:
> http://scientific.zcu.cz/fedora/rubygem-rails-controller-testing-0.1.1-2/
> rubygem-rails-controller-testing.spec
> SRPM URL:
> http://scientific.zcu.cz/fedora/rubygem-rails-controller-testing-0.1.1-2/
> rubygem-rails-controller-testing-0.1.1-2.fc25.src.rpm
> 
> %changelog
> * Mon Aug 08 2016 František Dvořák <valtri@xxxxxxxxxx> - 0.1.1-2
> - Pull request to include LICENSE file in the gem
> - Keep the tests in -doc subpackage

I APPROVED your code!

-- 
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
https://lists.fedoraproject.org/admin/lists/package-review@xxxxxxxxxxxxxxxxxxxxxxx




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