messagebird / ruby-rest-api

MessageBird's REST API for Ruby
BSD 2-Clause "Simplified" License
37 stars 46 forks source link

Wrapped List class inside MessageBird namespace (Solves issue #22) #23

Closed joris closed 2 years ago

epels commented 5 years ago

Hi @joris, thanks a lot for opening a pull request! Good catch, and I definitely agree List should be namespaced as well. We do however need to maintain backwards compatibility, and I believe this is a breaking change - is this correct?

joris commented 5 years ago

I think you’re right. I mean within the gem itself it doesn’t change anything because of how Ruby module nesting works. But indeed anyone using ::List directly, it will indeed break for them. So you might want to merge this into some future release. Though generally people should only upgrade gems explicitly anyways and re-test their app after a a gem upgrade. But we have of course zero control over that. To me it’s mostly important that it does get fixed. Because this is a bug in my mind (not an enhancement) and should therefore be fixed in the main gem at some point.

epels commented 5 years ago

@joris That is indeed the issue here. I do agree consumers should not update their dependencies without checking what has changed, but we can't assume this unfortunately. Even if we consider this a bug, there are people running this (successfully) in production right now, and changing this will break their code. I will set the milestone to v2 for this.

karloscodes commented 4 years ago

Any news about this?

lucaong commented 3 years ago

Is there a chance to merge this PR? I agree it's a breaking change, but only for people directly using the List class in their code (frankly very unlikely, as it's an internal utility class). On the other hand, it breaks any application that defines a global List class or module. As List is a pretty common name, one can expect that many applications could define such a class.

marcelcorso commented 2 years ago

hey @joris! Long time no see! 👋

This finally got merged and released here https://github.com/messagebird/ruby-rest-api/releases/tag/4.0.0

(I forgot to add it to the changelog 😞 )

thank you 🙏