Spec URL: http://data-strzibny.rhcloud.com/rubygem-strong_parameters.spec SRPM URL: http://data-strzibny.rhcloud.com/rubygem-strong_parameters-0.1.5-1.fc17.src.rpm Description: strong_parameters is an implementation of the slice pattern. The basic idea is to move mass-assignment protection out of the model and into the controller where it belongs. Fedora Account System Username: jstribny Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=4728639
Updated regarding Ruby 2 [1] and to the latest version: SPEC: http://data-strzibny.rhcloud.com/rubygem-strong_parameters.spec SRPM: http://data-strzibny.rhcloud.com/rubygem-strong_parameters-0.2.0-1.fc20.src.rpm Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=5287420 [1] https://fedoraproject.org/wiki/Features/Ruby_2.0.0
Hi Josef, I can take this review. Would you mind unmarking "tests" as %doc? Also it looks like this package should be updated for the latest Ruby guidelines. For example, you should run "gem unpack" and "gem spec" in %prep, "gem build" in %build, etc.
Also, please update to the latest upstream (0.2.1), and update the License field to "MIT".
Hi Ken, thanks for taking the review! > Would you mind unmarking "tests" as %doc? Sure, this is a mistake. > you should run "gem unpack" and "gem spec" in %prep, "gem build" in %build, etc. I will disagree with you here, %gem_install macro is completely sufficient and it's used this way in some gems. I am not patching anything, so those steps are redundant. But I moved it to %build, where it originally belongs. > update to the latest upstream Done. > and update the License field to "MIT" Good catch, changed. Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=5921913 SRPM: http://kojipkgs.fedoraproject.org//work/tasks/1914/5921914/rubygem-strong_parameters-0.2.1-1.fc19.src.rpm
(In reply to Josef Stribny from comment #4) > > you should run "gem unpack" and "gem spec" in %prep, "gem build" in %build, etc. > I will disagree with you here, %gem_install macro is completely sufficient > and it's used this way in some gems. I am not patching anything, so those > steps are redundant. > > But I moved it to %build, where it originally belongs. Hi Josef, Please help me understand this. On the Ruby Packaging Guidelines page, the example is clear about this, and gem2rpm's template also follows the unpack -> spec -> pack pattern. I thought the point of adding this in the Fedora 19 Ruby Packaging guidelines was that we would now be building the gem "from source". Why not propose a change to the packaging guidelines to fit your style?
> I thought the point of adding this in the Fedora 19 Ruby Packaging guidelines > was that we would now be building the gem "from source". Maybe. Maybe it's there to match other guidelines better? The package doesn't come with any compiled source, and `gem install` actually compiles the extension if there is any (gems should come with sources, not compiled code) and before that, it actually also unpacks the .gem file. > Why not propose a change to the packaging guidelines to fit your style? I believe this was already discussed before the new guidelines were accepted. Besides the guidelines are a good default, which I don't want to change. I just prefer to simplify things where it's possible. But for piece of mind, I made the change for you: Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=5925972 SRPM: http://kojipkgs.fedoraproject.org//work/tasks/5973/5925973/rubygem-strong_parameters-0.2.1-2.fc19.src.rpm
(In reply to Josef Stribny from comment #6) > But for piece of mind, I made the change for you: Thank you - that is really great :) I've completed the review, and there are two remaining issues. The first is really minor. In %install, please use "cp -pa" rather than "cp -a" in order to preserve the file timestamps. The second is more complex. This gem is not compatible with Rails 4. If I remove the "< 4" version restrictions in the .spec file, and try to build it for Rawhide, the test suite fails: Executing(%check): /bin/sh -e /var/tmp/rpm-tmp.UAJLf3 + umask 022 + cd /builddir/build/BUILD ~/build/BUILD/strong_parameters-0.2.1/usr/share/gems/gems/strong_parameters-0.2. 1 ~/build/BUILD/strong_parameters-0.2.1 + cd strong_parameters-0.2.1 + pushd ./usr/share/gems/gems/strong_parameters-0.2.1 + testrb -Ilib test/action_controller_required_params_test.rb test/action_contro ller_tainted_params_test.rb test/active_model_mass_assignment_taint_protection_t est.rb test/controller_generator_test.rb test/log_on_unpermitted_params_test.rb test/multi_parameter_attributes_test.rb test/parameters_permit_test.rb test/para meters_require_test.rb test/parameters_taint_test.rb test/raise_on_unpermitted_p arams_test.rb /builddir/build/BUILD/strong_parameters-0.2.1/usr/share/gems/gems/strong_paramet ers-0.2.1/lib/action_controller/parameters.rb:11:in `<module:ActionController>': superclass mismatch for class ParameterMissing (TypeError) This error led me to: https://github.com/rails/strong_parameters/issues/45 What does that mean for this package? Is this functionality already in Rails 4?
(In reply to Ken Dreyer from comment #7) > This error led me to: > > https://github.com/rails/strong_parameters/issues/45 > > What does that mean for this package? Is this functionality already in Rails > 4? This gem is just for Rails 3: https://github.com/rails/strong_parameters#compatibility Integrated somehow into Rails 4 since last September: https://github.com/rails/rails/commit/885005461b3cc0d073ec08495dc3bf06d0bebf2a The independent strong_parameters gem was probably required by RoR at some point, but I can't find where/how. We should probably close this review.
> The independent strong_parameters gem was probably required by RoR at some > point, but I can't find where/how. We should probably close this review. True, closing.