Since hashie/mash can be required alone, require its dependencies #368

jrafanie · 2016-09-16T14:57:42Z

Fixes #367

While it's probably not something hashie wants to allow, previously
this was possible so we shouldn't break this when going 3.4.4 to 3.4.5.

As an example, omniauth requires hashie/mash alone

This appears to have been broken in #358.

Note, we can't write a test for this because we require 'hashie' in the spec_helper
which properly defines the Hashie::Extensions::Array constant and autoloads
the pretty_inspect here

Before

10:44:30 ~/Code/hashie (master) (2.3.1) + be irb
irb(main):001:0> require 'hashie/mash'
NameError: uninitialized constant Hashie::Extensions::Array
Did you mean?  Hashie::Array
               Array
  from /Users/joerafaniello/Code/hashie/lib/hashie/array.rb:3:in `<class:Array>'
  from /Users/joerafaniello/Code/hashie/lib/hashie/array.rb:2:in `<module:Hashie>'
  from /Users/joerafaniello/Code/hashie/lib/hashie/array.rb:1:in `<top (required)>'
  from /Users/joerafaniello/Code/hashie/lib/hashie/mash.rb:2:in `require'
  from /Users/joerafaniello/Code/hashie/lib/hashie/mash.rb:2:in `<top (required)>'
  ...

After

10:44:40 ~/Code/hashie (master) (2.3.1) + be irb
irb(main):001:0> require 'hashie/mash'
=> true
irb(main):002:0> exit

dblock · 2016-09-16T14:59:37Z

Personally, I am against this. We're working around people requiring hashie/mash directly and removing autoload. Most classes in hashie cannot be required this way and it's an accident that this one worked.

dblock · 2016-09-16T15:01:43Z

But looking at the codebase I see that we're doing similar things in trash, https://github.com/intridea/hashie/blob/master/lib/hashie/trash.rb#L2.

@jrafanie Can you please update CHANGELOG? I think we don't need anything in UPGRADING, I'll just cut another release after this is merged/built/tested.

jrafanie · 2016-09-16T15:02:42Z

@jrafanie Can you please update CHANGELOG?

Ok, will do

jrafanie · 2016-09-16T15:11:10Z

Done @dblock. While I agree, this is less than ideal, maybe you can make a checklist of things we want to enforce in the next breaking "release" and add this to the list. If we're using autoload everywhere and want to enforce that, let's do it with a changelog entry in 3.5 or 4.0. omniauth is a really popular library so you're going to get lots of grief from people hitting this.

Fryguy · 2016-09-16T15:13:57Z

CHANGELOG.md

@@ -28,7 +28,7 @@ scheme are considered to be bugs.

 ### Fixed

-* Your contribution here.
+* [#368](https://github.com/intridea/hashie/pull/368): hashie/mash can be required alone, require it's dependencies - [@jrafanie](https://github.com/jrafanie/)


Typo it's -> its

Also add a period at the end of this, quote hashie/mash and remove the trailing slash from jrafanie/ ;)

Thanks, should be be correct now. 🙇

litch · 2016-09-16T15:15:13Z

I understand wanting to remove undesirable parts of your API, but this seems to have broken FaradayMiddleware::Mashify (https://github.com/lostisland/faraday_middleware/blob/master/lib/faraday_middleware/response/mashify.rb), so probably deprecating before breaking changes would be helpful.

dblock · 2016-09-16T15:17:27Z

@jrafanie if you think something should be checklisted for a release that needs to go into RELEASING.md (PR as usual) and needs to be easy to do.

Fixes hashie#367 While it's probably not something hashie wants to allow, previously this was possible so we shouldn't break this when going 3.4.4 to 3.4.5. As an example, omniauth requires hashie/mash alone: https://github.com/omniauth/omniauth/blob/d941c4bcb1eb52f3674dd46225808751dd6a1c2f/lib/omniauth/auth_hash.rb#L1 This appears to have been broken in hashie#358. Note, we can't write a test for this because we require 'hashie' in the spec_helper which properly defines the Hashie::Extensions::Array constant and autoloads the pretty_inspect here: https://github.com/intridea/hashie/blob/229ee36d7c6a07eff6d8a0434726e12b8c3a0223/lib/hashie.rb#L48 **Before** ``` 10:44:30 ~/Code/hashie (master) (2.3.1) + be irb irb(main):001:0> require 'hashie/mash' NameError: uninitialized constant Hashie::Extensions::Array Did you mean? Hashie::Array Array from /Users/joerafaniello/Code/hashie/lib/hashie/array.rb:3:in `<class:Array>' from /Users/joerafaniello/Code/hashie/lib/hashie/array.rb:2:in `<module:Hashie>' from /Users/joerafaniello/Code/hashie/lib/hashie/array.rb:1:in `<top (required)>' from /Users/joerafaniello/Code/hashie/lib/hashie/mash.rb:2:in `require' from /Users/joerafaniello/Code/hashie/lib/hashie/mash.rb:2:in `<top (required)>' ... ``` **After** ``` 10:44:40 ~/Code/hashie (master) (2.3.1) + be irb irb(main):001:0> require 'hashie/mash' => true irb(main):002:0> exit ```

jrafanie · 2016-09-16T15:30:53Z

@jrafanie if you think something should be checklisted for a release that needs to go into RELEASING.md (PR as usual) and needs to be easy to do.

@dblock the release process looks good 👍 . This is not something a release process could have caught easily. Breakages happen.

All green now. 🎉

This can be reverted when hashie/hashie#368 is merged and released.

dblock · 2016-09-16T17:45:02Z

Merged and released 3.4.6. Please double check that everything works now.

dblock · 2016-09-16T17:48:44Z

@jrafanie Thanks for your help!

jrafanie · 2016-09-16T18:30:56Z

Thanks @dblock 🙇

@jrafanie

[3.4.6]: hashie/hashie@v3.4.5...v3.4.6 ### Fixed * [#368](hashie/hashie#368): Since `hashie/mash` can be required alone, require its dependencies - [@jrafanie](https://github.com/jrafanie). ## [3.4.5] - 2016-09-16 [3.4.5]: hashie/hashie@v3.4.4...v3.4.5 ### Added * [#337](hashie/hashie#337), [#331](hashie/hashie#331): `Hashie::Mash#load` accepts a `Pathname` object - [@gipcompany](https://github.com/gipcompany). ### Deprecated * [#366](hashie/hashie#366): Hashie is no longer tested on Ruby < 2 - [@dblock](https://github.com/dblock). ### Fixed * [#358](hashie/hashie#358): Fixed support for Array#dig - [@modosc](https://github.com/modosc). * [#365](hashie/hashie#365): Ensured ActiveSupport::HashWithIndifferentAccess is defined before use in #deep_locate - [@mikejarema](https://github.com/mikejarema). ### Miscellanous * [#366](hashie/hashie#366): Added Danger, PR linter - [@dblock](https://github.com/dblock). ## [3.4.4] - 2016-04-29 [3.4.4]: hashie/hashie@v3.4.3...v3.4.4 ### Added * [#349](hashie/hashie#349): Convert `Hashie::Mash#dig` arguments for Ruby 2.3.0 - [@k0kubun](https://github.com/k0kubun). ### Fixed * [#240](hashie/hashie#240): Fixed nesting twice with Clash keys - [@bartoszkopinski](https://github.com/bartoszkopinski). * [#317](hashie/hashie#317): Ensure `Hashie::Extensions::MethodQuery` methods return boolean values - [@michaelherold](https://github.com/michaelherold). * [#319](hashie/hashie#319): Fix a regression from 3.4.1 where `Hashie::Extensions::DeepFind` is no longer indifference-aware - [@michaelherold](https://github.com/michaelherold). * [#322](hashie/hashie#322): Fixed `reverse_merge` issue with `Mash` subclasses - [@marshall-lee](https://github.com/marshall-lee). * [#346](hashie/hashie#346): Fixed `merge` breaking indifferent access - [@docwhat](https://github.com/docwhat), [@michaelherold](https://github.com/michaelherold). * [#350](hashie/hashie#350): Fixed from string translations used with `IgnoreUndeclared` - [@marshall-lee](https://github.com/marshall-lee).

jrafanie mentioned this pull request Sep 16, 2016

NameError: uninitialized constant Hashie::Extensions::Array #367

Closed

jrafanie force-pushed the fix_name_error_hashie_extensions_array branch from b460601 to e89ac50 Compare September 16, 2016 15:07

Fryguy reviewed Sep 16, 2016

View reviewed changes

jrafanie force-pushed the fix_name_error_hashie_extensions_array branch from e89ac50 to 66386b3 Compare September 16, 2016 15:18

jrafanie changed the title ~~hashie/mash can be required alone, require it's dependencies~~ Since hashie/mash can be required alone, require its dependencies Sep 16, 2016

jrafanie force-pushed the fix_name_error_hashie_extensions_array branch from 66386b3 to 0feba44 Compare September 16, 2016 15:21

md5 mentioned this pull request Sep 16, 2016

Fix Travis builds using appraisal gem omniauth/omniauth-saml#113

Merged

carbonin added a commit to carbonin/manageiq that referenced this pull request Sep 16, 2016

Lock hashie gem to the last working version

ae53c98

This can be reverted when hashie/hashie#368 is merged and released.

carbonin mentioned this pull request Sep 16, 2016

Lock hashie gem to the last working version ManageIQ/manageiq#11327

Merged

dblock merged commit 21c9ea2 into hashie:master Sep 16, 2016

jrafanie deleted the fix_name_error_hashie_extensions_array branch September 16, 2016 18:30

dblock mentioned this pull request Jan 31, 2017

New version of hashie breaks omniauth #391

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Since hashie/mash can be required alone, require its dependencies #368

Since hashie/mash can be required alone, require its dependencies #368

jrafanie commented Sep 16, 2016 •

edited

dblock commented Sep 16, 2016

dblock commented Sep 16, 2016 •

edited

jrafanie commented Sep 16, 2016

jrafanie commented Sep 16, 2016

Fryguy Sep 16, 2016

jrafanie Sep 16, 2016

dblock Sep 16, 2016

jrafanie Sep 16, 2016

litch commented Sep 16, 2016

dblock commented Sep 16, 2016 •

edited

jrafanie commented Sep 16, 2016

dblock commented Sep 16, 2016

dblock commented Sep 16, 2016

jrafanie commented Sep 16, 2016

Since hashie/mash can be required alone, require its dependencies #368

Since hashie/mash can be required alone, require its dependencies #368

Conversation

jrafanie commented Sep 16, 2016 • edited

dblock commented Sep 16, 2016

dblock commented Sep 16, 2016 • edited

jrafanie commented Sep 16, 2016

jrafanie commented Sep 16, 2016

Fryguy Sep 16, 2016

Choose a reason for hiding this comment

jrafanie Sep 16, 2016

Choose a reason for hiding this comment

dblock Sep 16, 2016

Choose a reason for hiding this comment

jrafanie Sep 16, 2016

Choose a reason for hiding this comment

litch commented Sep 16, 2016

dblock commented Sep 16, 2016 • edited

jrafanie commented Sep 16, 2016

dblock commented Sep 16, 2016

dblock commented Sep 16, 2016

jrafanie commented Sep 16, 2016

jrafanie commented Sep 16, 2016 •

edited

dblock commented Sep 16, 2016 •

edited

dblock commented Sep 16, 2016 •

edited