search

google / smali

Other
175 stars 29 forks source link

Breaking change introduced with a refactor #58

Closed oSumAtrIX closed 2 weeks ago

oSumAtrIX commented 1 month ago

With commit bad94743a0bd5a1d3bce4062fa539bba28f68d9c, a breaking API change was introduced. The breaking change occurs in the signature of a method part of the public API: https://github.com/google/smali/commit/bad94743a0bd5a1d3bce4062fa539bba28f68d9c#diff-1b64ab2237880ceb837437aa949dca5004f305bd0aac42fe8f6a67b3d6e971d6R60.

A deprecated overload can be added until the next major bump.

melcz commented 3 weeks ago

Adding the deprecated version back would require depending on Guava, which is what we're trying to avoid. ImmutableList is an implementation of List, so even if the method signature changed, equivalent read-only usage should be possible.

oSumAtrIX commented 3 weeks ago

Java binary compatibility is broken as the types have changed. The idea would be to deprecate the APIs that rely on GUAVA and introduce new APIs using the List type at the same time. Eventually you can remove the deprecated APIs with a major version bump. Right now we are forced to update this library synchronized in two projects due to the breaking API change, but I can understand the overhead involved in avoiding that with deprecation. Feel free to close this issue as not planned.

melcz commented 2 weeks ago

Thanks for bringing this up. Breaking compatibility is an unfortunate effect of this refactor. However, adding back the immutable collections dependencies will require patching the library when importing into Android Platform, which means more maintenance work. So for this time we'll choose the inconveniences of the broken binary compatibility given that most cases should be like the one you highlighted (ImmutableList -> List) and are straightforward to work around. Apologies for the inconvenience!