Bug 967396 - Review Request: rubygem-chosen-rails - Integrate Chosen JavaScript library with Rails asset pipeline
Summary: Review Request: rubygem-chosen-rails - Integrate Chosen JavaScript library wi...
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
(Show other bugs)
Version: rawhide
Hardware: All Linux
medium
medium
Target Milestone: ---
Assignee: Vít Ondruch
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2013-05-27 03:14 UTC by Anuj More
Modified: 2016-01-04 09:01 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-01-04 09:01:19 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

Description Anuj More 2013-05-27 03:14:22 UTC
Spec URL: http://anujmore.fedorapeople.org/pkgs/rubygem-chosen-rails/rubygem-chosen-rails.spec
SRPM URL: http://anujmore.fedorapeople.org/pkgs/rubygem-chosen-rails/rubygem-chosen-rails-0.9.14-1.fc19.src.rpm
Description: Integrate Chosen JavaScript library with Rails asset pipeline
Fedora Account System Username: anujmore

Comment 1 Anuj More 2013-05-27 03:15:43 UTC
Mock builds successfully: https://raw.github.com/execat/Packages/master/rpmspecs/rubygem-chosen-rails/mock
rpmlint complains (but barely ;-)):
rubygem-chosen-rails.noarch: W: spelling-error %description -l en_US jQuery -> j Query, query, equerry
rubygem-chosen-rails.src: W: spelling-error %description -l en_US jQuery -> j Query, query, equerry
3 packages and 0 specfiles checked; 0 errors, 2 warnings.

Comment 2 Josef Stribny 2013-05-28 13:56:52 UTC
* LICENSE should be marked as %doc
* Gemfile and Rakefile should be in the -doc sub-package rather than excluded
* README.md should be moved to the -doc sub-package as it's not require during run-time

Other than that I tested it with a simple generated app and everything seems to be fine. Please fix the issues above and I will approve.

Comment 3 Anuj More 2013-05-29 06:09:57 UTC
Hi Josef,

Thanks for reviewing the package.

I am following these guidelines (set by myself after looking at example specs and talking to vondruch): https://github.com/execat/Packages/blob/master/rpmspecs/packaging_rules

Changing these would mean changing *ALL* the specfiles I have submitted and am currently working on (There are 20 of those, so that could take some time).

About Gemfile and Rakefile, vondruch specifically asked me to remove them all unless something in the Rakefile really doesn't let package to function properly.

(Applies to https://bugzilla.redhat.com/show_bug.cgi?id=967333 as well)

Comment 4 Vít Ondruch 2013-05-29 09:52:08 UTC
(In reply to Anuj More from comment #3)
> Hi Josef,
> 
> Thanks for reviewing the package.
> 
> I am following these guidelines (set by myself after looking at example
> specs and talking to vondruch):
> https://github.com/execat/Packages/blob/master/rpmspecs/packaging_rules

cp -pr spec/ %{buildroot}%{gem_instdir}
cp -pr spec/ %{buildroot}%{gem_instdir}

I would suggest to replace the %{buildroot} with '.', i.e. wit the current directory.

Actually, I am not sure why exactly we wrote the guidelines that way, since unpacking the test suite in %check section would do the same (something like in activemodel) on one line. We will try to clarify this with FPC.

> About Gemfile and Rakefile, vondruch specifically asked me to remove them
> all unless something in the Rakefile really doesn't let package to function
> properly.

I agree with jstribny. I am not sure I would said that anytime. I may said, that some maintainer might have different opinion, but I typically try to keep everything what comes in gem packaged. If I want to remove something, then I ask upstream to do that.

[1] http://pkgs.fedoraproject.org/cgit/rubygem-activemodel.git/tree/rubygem-activemodel.spec#n66

Comment 6 Vít Ondruch 2016-01-04 09:01:19 UTC
Closing this stalled review.


Note You need to log in before you can comment on or make changes to this bug.