Bug 1264661

Summary: Review Request: rubygem-generator_spec - Test Rails generators with RSpec
Product: [Fedora] Fedora Reporter: Ilia Gradina <ilya.gradina>
Component: Package ReviewAssignee: Vít Ondruch <vondruch>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: package-review, vondruch
Target Milestone: ---Flags: vondruch: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-10-20 11:50:57 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1267323, 1268691    

Description Ilia Gradina 2015-09-19 23:49:16 UTC
Spec URL: https://github.com/ilgrad/fedora-packages/raw/master/rubygems/rubygem-generator_spec.spec
SRPM URL: https://github.com/ilgrad/fedora-packages/raw/master/rubygems/rubygem-generator_spec-0.9.3-1.fc24.src.rpm
Description: Test Rails generators with RSpec
Fedora Account System Username: ilgrad

Comment 1 Vít Ondruch 2015-09-25 11:28:15 UTC
Hi Ilya,

does it actually build in Koji? I am asking since I see BR: rubygem(bundler) but no other treatment. This typically means that some dependencies are pulled in during build, but this is not possible on Koji, since it has no network access.

Comment 2 Upstream Release Monitoring 2015-09-29 09:26:49 UTC
ilgrad's scratch build of rubygem-generator_spec-0.9.3-1.fc24.src.rpm for f24 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11264561

Comment 3 Ilia Gradina 2015-09-29 09:45:57 UTC
Hi Vit,

everything seems fine with this package. What will be next steps?

Comment 4 Vít Ondruch 2015-09-29 10:05:58 UTC
Ok, thx. I'll take it for a review.

Comment 5 Vít Ondruch 2015-09-29 10:38:46 UTC
* Consider excluding dot file using wildcard
  - This is not a blocker, but you might want to exclude all the dot files in
    once using:

      %exclude %{gem_instdir}/.*

    Of course this might be rather big hammer, so this is just for your
    consideration.

* Exclude .gemspec
  - Since the .gemspec you are including in the -doc subpackage is not the
    original one (it is the rewritten version created in %prep section), I would
    suggest to simply exclude the file from package.

* Avoid Bundler dependency
  - Although the Bundler is working just OK in this case, I would still
    recommend to avoid this dependency. It is as easy as modify the %check
    section in this way:

      sed -i "/require 'bundler/ s/^/#/" spec/spec_helper.rb
      rspec -rpathname -Ilib spec

    This might help in case that Fedora is bootstrapped for example. You can
    build this package sooner and possibly avoid some circular dependencies etc.

* Remove unnecessary dependencies
  - The BR: rubygem(activesupport) is not really needed, since it is pulled in
    via rubygem(railties) dependency.
  - In case you decide to avoid the Bundler dependency, you can remove
    BR: rubygem(bundler) as well.


Nevertheless, since all the above are just minor nits, I APPROVE the package.

Comment 6 Ilia Gradina 2015-09-29 13:16:09 UTC
Thank you Vit!

I will try to continue to consider these comments.

Spec URL: http://repo.clanwars.org/gitlab/rubygem-generator_spec.spec
SRPM URL: http://repo.clanwars.org/gitlab/rubygem-generator_spec-0.9.3-2.fc24.src.rpm

Comment 7 Upstream Release Monitoring 2015-09-29 13:20:26 UTC
ilgrad's scratch build of rubygem-generator_spec-0.9.3-2.fc24.src.rpm for f24 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11266041

Comment 8 Vít Ondruch 2015-09-30 08:08:19 UTC
(In reply to Ilya Gradina from comment #6)
> Spec URL: http://repo.clanwars.org/gitlab/rubygem-generator_spec.spec

Looks perfect now! Thx.

Comment 9 Vít Ondruch 2015-09-30 08:11:29 UTC
BTW it seems that you are looking for more reviews. It is probably worth to try to ask for review swap on fedora-devel or ruby-sig ML ... or probably both ;) And you can also finish your informal reviews (if there is some response).

Comment 10 Vít Ondruch 2015-10-05 08:17:25 UTC
Ilya, this package was approved, you can continue with scm request ....

Comment 11 Ilia Gradina 2015-10-05 08:34:55 UTC
Hi Vit,

I was refused.
https://admin.fedoraproject.org/pkgdb/packager/ilgrad/requests

Comment 12 Vít Ondruch 2015-10-05 09:50:15 UTC
(In reply to Ilya Gradina from comment #11)
I asked pingou on IRC about this issue and there seems to be bug somewhere:

-pingou- vondruch: seems to be a bug in the script: ! User upstrea
>pingou< m-release-monitoring is not a packager
 the person running the script shouldn't have had a problem with this...
>vondruch< pingou, ok ... and the solution is?
-pingou- vondruch: asking again sounds like the correct solution
>vondruch< pingou, ok ... he did that, so hopefully it will pass next time :)
-pingou- vondruch: I removed the old entry, I know there is a bug in this part of the code
* pingou working on fixing it

tl;dr you don't need to do anything ATM and it should hopefully pass next time.

Comment 13 Vít Ondruch 2015-10-20 11:50:57 UTC
Ilya, please close the review tickets once you import and build the package. Thx.