In addition to the rule of the patch releases, it's acceptable to have new public methods for bug fixes, though we should pay enough effort to avoid it. If so, we should not remove or change any public method.
I just checked a few recent PRs in branch-3.0 and added some examples here. https://github.com/apache/pulsar/pull/20733 Instead of removing WebSocketPingPongServlet and PingPongHandler, we should mark them as deprecated once we agree to my proposal here. https://github.com/apache/pulsar/pull/20666 It's not acceptable even for now because it changes the public API org.apache.pulsar.broker.loadbalance.extensions.filter.BrokerFilter. It's also a bad example. https://github.com/apache/pulsar/pull/20677 It's not acceptable that the static `TopicCompactionStrategy#load` method signature changed. #20666 and #20677 are also bad examples that the public APIs are not designed well in the PIP-192 proposal. The methods are not designed well so the author modify them casually. Thanks, Yunze On Fri, Jul 14, 2023 at 6:52 PM Yunze Xu <x...@apache.org> wrote: > > Hi all, > > As the Pulsar community is growing, the core project could be depended > on by many ecosystem projects, including protocol handlers and > connectors. However, there is no clear API compatibility guarantee at > the moment. > > For now, PIP is required when public APIs change [1]. However, I think > the public API only refers to the interfaces in: > - The pulsar-client-api module > - The pulsar-admin-api module > - Any plugin interface, e.g. ProtocolHandler > > However, when a 3rd party project adds Pulsar as the dependency, it's > legal and common to use any public class and any public method from > it. For a specific example, just as I've discussed many times, the > MessageId interface from the pulsar-client-api module, the 3rd party > project tends to cast it to the specific implementation (e.g. > MessageIdImpl) to use.Then the challenge comes, if we modifies any > public method of MessageIdImpl, the ecosystem developer could suffer > from it. > > For this case, we have no clear rule on how Pulsar guarantees the API > compatibility. So I'd like to propose my suggestion here, once it > reaches the consensus, I will write a formal proposal and add it to > our contribution guide [2]. > > Here are my proposed rules (I will add specific examples for a formal > proposal): > 1. For any new interface, add the > org.apache.pulsar.common.classification.InterfaceStability > 2. For patch releases, e.g. 3.0.x to 3.0.y, we should never introduce > any public method change. > 3. For minor releases, e.g. 3.1.0 to 3.2.0, when you want to modify a > public method, e.g. `foo(int, int)` is changed to `foo(int, String)`, > don't remove the original `foo(int, int)` method. Instead, add the > `@Deprecated` annotation and keep the semantics not changed if > possible. > 4. Any public method with the `@Deprecated` annotation can be removed > in the next minor release. > 5. The rules above are applied to all modules, not only the xxx-api modules. > > With the rules above, > - For ecosystem developers, they can know the risk to use any public > method from a class, especially implementation class. They can also be > confident that no API will change if they upgrade Pulsar from a.b.c to > a.b.d. > - For committers, they should be more careful to review the PRs that > introduce API changes. > - For release managers, it would be clear that any PR that changes any > public method should not be cherry-picked to release branches. > > [1] https://github.com/apache/pulsar/tree/master/pip#when-is-a-pip-required > [2] https://pulsar.apache.org/contribute/ > > Thanks, > Yunze
