-
-
Notifications
You must be signed in to change notification settings - Fork 242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow addressable 2.4+ to work with json-schema #312
Conversation
The catch with this change is that addressable 2.4.0 produces a load of ruby warnings (again) |
9505f4d
to
f5e07d5
Compare
👍 |
@RST-J there are two catches with this PR:
I'd like to fix 2 at some point, but how do you feel about 1? |
I don't mind to force a dependency to be updated as long it does not silently break other places. |
I'm afraid that there's no way to do that. If we merge this change, anyone updating json schema would also have to update addressable (throughout their project). It might be worth holding off on merging this until there is at least addressable 2.4.1 (ie when other gems have had more time to upgrade as well) |
I mean, as long as it is clear that upgrading json-schema requires upgrading addressable, people may stick with the current version, if they cannot upgrade addressable for whatever reason. |
I've played with this In fact I got it to a smaller minimum a = Addressable::URI.parse('#')
b = Addressable::URI.parse("http://google.com")
a.defer_validation do
a.merge!(b)
a.fragment = ""
end |
@iainbeeston it does not look like there will be 2.4.1 anytime soon. There is only one commit on master since the release of 2.4.0. Anyways, it is out for more than half a year now. I think, it is reasonable to merge this. Although we could wait until we make our next minor release. And maybe you find a way to fix the warning thing until then? |
Yes, you're right - we should look at doing this. I'd still like to try to resolve that warning first though. I'll look at this again soon |
Would be great if you could merge this, I'm currently having dependency issues in a project due to some other gems requiring Addressable >= 2.4. |
Yes, sorry, addressable 2.4 has been out for a while now... |
I've submitted a fix for the warning messages to addressable sporkmonger/addressable#244 |
I've had to change one of the specs because of this. It seems that addressable now raises an error if you parse a uri that looks like a json text, which was one of our test cases. This fixes and supersedes voxpupuli#301
f5e07d5
to
d3a557d
Compare
For anyone who is waiting for this - I've just released json-schema 2.7, which requires addressable 2.4+ |
I've had to change one of the specs because of this. It seems that
addressable now raises an error if you parse a uri that looks like a
json text, which was one of our test cases.
This fixes and supersedes #301