Thank you. I will aim for submitting a PR over the coming weekend.
On Mon, Aug 10, 2015 at 1:18 PM, Rafael Mendonça França < [email protected]> wrote: > I think it is worth to experiment yes, I don't want to decline anything > without seeing the code and how it would feel. I can see advantages of > doing that. > > On Mon, Aug 10, 2015 at 2:11 PM <[email protected]> wrote: > >> Rafael, >> >> I made 3 strong arguments in response to your point about capybara. I >> would have appreciated a response explaining if any of my reasoning is >> wrong here. >> >> This is not a difficult PR and I still feel the case for it is strong but >> I don't want to start working on it while I'm waiting to hear from you. >> >> >> On Monday, July 13, 2015 at 6:37:28 PM UTC-4, [email protected] wrote: >>> >>> Rails goes out of its way to avoid forcing an installation of bcrypt >>> because it is a binary library. See >>> https://github.com/rails/rails/blob/v4.2.3/Gemfile#L21 >>> >>> Nokogiri forces installation of 2 binary libraries (libxml2 and >>> libxslt), so one would expect it not to be a dependency of any of the core >>> components of Rails. >>> >>> However, starting with actionview 4.2.0, nokogiri is now a dependency. >>> >>> That means every time actionview appears in a Gemfile.lock, so does >>> nokogiri. I would often include ActionView 4.1 in non-Rails projects just >>> to use number_to_currency, but now with the nokogiri dependency, the >>> overhead is hardly worth it. >>> >>> Consider the fact that I'm deploying about 5 such projects to the same >>> server, all using separate BUNDLE_PATH's. That means 5 installations of >>> nokogiri, none of which are being used. This adds time to every >>> `capistrano bundler:install` and a significant amount of disk space is >>> wasted on this. For any other gem, this wouldn't make much of a >>> difference, but nokogiri is really big and takes a long time to install, >>> and Rails has already set a precedent by not including the (much lighter) >>> bcrypt. >>> >>> Is the rails-core team open to the following solutions: >>> --------------- >>> >>> 1) Separate the parts of actionview that depend on rails-dom-testing >>> into a separate gem >>> >>> Create an actionview-testing gem, which would only be necessary in the >>> Gemfile's test group, thus saving even more overhead in production. This >>> would depend on action-view and rails-dom-testing, but actionview would not >>> depend on rail-dom-testing. >>> >>> (The same approach that I suggest below for rails-html-sanitizer might >>> work for rails-dom-testing too, but it may add more complexity that carving >>> a separate gem--there are multiple code paths that can lead you to >>> rails-dom-testing methods, whereas there's a single method that's a >>> bottleneck for all entries to rails-html-sanitizer.) >>> --------------- >>> >>> 2) In ActionView::Helpers::SanitizeHelper, move `require >>> "rails-html-sanitizer"` into the #sanitizer_vendor method. >>> >>> If a LoadError is raised, handle it just as we do for bcrypt: >>> https://github.com/rails/rails/blob/v4.2.3/activemodel/lib/active_model/secure_password.rb#L60 >>> >>> Add rails-html-sanitizer to the Gemfile template so that it's >>> automatically in new Rails projects. Only upgrades and would need to >>> manually add this to the Gemfile, so we would have to add it to the upgrade >>> guide. Standalone actionview projects would also need to add it to their >>> Gemfile, but *rafaelfranca <https://github.com/rafaelfranca>* has >>> assured me that non-Rails projects should never be using >>> rails-html-sanitizer anyway: >>> https://github.com/rails/rails-html-sanitizer/issues/25#issuecomment-60833972 >>> . >>> --------------- >>> >>> I would love to get started on a PR. I just need to know if it will be >>> considered. >>> >>> >>> >>> >>> >>> -- >> You received this message because you are subscribed to the Google Groups >> "Ruby on Rails: Core" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> To post to this group, send email to [email protected]. >> Visit this group at http://groups.google.com/group/rubyonrails-core. >> For more options, visit https://groups.google.com/d/optout. >> > -- > You received this message because you are subscribed to a topic in the > Google Groups "Ruby on Rails: Core" group. > To unsubscribe from this topic, visit > https://groups.google.com/d/topic/rubyonrails-core/TqPPisZ3ttU/unsubscribe > . > To unsubscribe from this group and all its topics, send an email to > [email protected]. > To post to this group, send email to [email protected]. > Visit this group at http://groups.google.com/group/rubyonrails-core. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.
