Hi Till, > Personally, I'd be fine to say that in API modules (tbd what this is > (probably transitive closure of all APIs)) we require every class to be > annotated.
At least we'll then need the reverse rule: no classes outside *.api.* packages CAN have an API annotation (other than Internal), of course with many existing violations that need to be accapted. That's still a much weaker requirement, though, as classes can just be left unannotated, which is why I prefer annotating all classes regardless of location. > If we have cases that violate the guideline, then I think we either have to > remove these methods How would you handle e.g. extending an existing Public interface with a new method in this case, though? You'd be forced to immediately make the new method Public as well, or place it somewhere else entirely, which leads to unfavorable design. I don't think we should disallow extending classes with methods of a weaker stability. Best Ingo On Fri, Dec 3, 2021 at 4:53 PM Till Rohrmann <trohrm...@apache.org> wrote: > Hi Ingo, thanks a lot for your thoughts and the work you've spent on this > topic already. > > Personally, I'd be fine to say that in API modules (tbd what this is > (probably transitive closure of all APIs)) we require every class to be > annotated. > > For sake of clarity and having clear rules, this should then also apply to > nested types. As you've said this would have some additional benefits when > doing refactorings and seems to be actually required by japicmp. > > >This seems to be quite incompatible with the current interpretation in the > code base, and it would prevent valid (and in-use) use cases like marking > e.g. a single method experimental (or even internal) in an otherwise public > (evolving) API. > > If we have cases that violate the guideline, then I think we either have to > remove these methods or adjust their stability guarantees (time of > reckoning ;-). Otherwise, we won't be able to give stability guarantees, I > fear. > > Cheers, > Till > > On Fri, Dec 3, 2021 at 4:23 PM Ingo Bürk <i...@ververica.com> wrote: > > > Hi Till, > > > > Thanks for starting this discussion, and as you noticed, I qutie care for > > it as well. :-) > > > > When implementing the ArchUnit rules, I noticed that project-wide, > > different teams / modules use and understand the API annotations in > > different ways. Therefore, I think it is important to both get clarity on > > the meaning of these annotations (= your FLIP), but also on how to > actually > > use them. > > > > One thing I would like to bring up for discussion is requiring an API > > annotation on every class. This seems to be a controversial suggestion, > > though personally I think it would allow our tooling do its job best. An > > obvious downside is "it's annoying", but so is the current requirement of > > having JavaDocs on every class (including tests). If this were to ever be > > considered, this should be implemented (also) in Checkstyle, though, to > > have it as immediate IDE feedback, which IMO would also mostly eliminate > > this downside. The other downside is that it encourages throwing > > Public(Evolving) around, but that would really only make an existing > > invisible problem actually visible; ultimately, I think this is something > > that committers just need to consider. > > It would be nice to only require this in *.api.* packages, but that's > just > > not the world we live in, nor will it be anytime soon; however, it would > > allow us to make sure that new non-internal APIs are only added in > *.api.* > > packages, or that no public APIs are added in *.internal.* packages. > > > > Another aspect that came up both when implementing the rules and again > in a > > discussion is whether (or when) nested types (classes, interfaces – > > not members) need to be annotated explicitly. My understanding[1] is that > > this is actually needed for japicmp to work correctly. Besides that, I > > think doing so adds benefits such as making the breaking change visible > > when promoting a nested type to a top-level type, which would likely go > > unquestioned if the type is not annotated but "inherited" its API > > stability, losing it in the process. > > > > Now to actually comment on your FLIP: > > > > > In order to determine the stability guarantee a given class/interface > can > > provide, we have to take a look at all methods a user has to implement > and > > use for using the given class/interface. The weakest guarantee of these > > methods specifies the guarantee we can give for the containing > > class/interface. > > > > This seems to be quite incompatible with the current interpretation in > the > > code base, and it would prevent valid (and in-use) use cases like marking > > e.g. a single method experimental (or even internal) in an otherwise > public > > (evolving) API. > > > > [1] https://github.com/apache/flink/pull/17133#issuecomment-925738694 > > > > > > Best > > Ingo > > > > On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <trohrm...@apache.org> > wrote: > > > > > Hi everyone, > > > > > > I would like to start a discussion about what kind of API stability > > > guarantees we want to provide to our users. The Flink community already > > > agreed on some stability guarantees, but these guarantees were only > > > communicated via the ML and not properly documented [2]. Moreover, I've > > > seen more and more complaints about breaking changes (some rightful and > > > others not) on the ML recently [3] because we rarely mark our APIs as > > > stable. This motivated this FLIP. > > > > > > The proposal first concentrates on source API stability guarantees. > > Binary > > > stability guarantees are explicitly left out for a follow-up > discussion. > > > > > > In essence, the proposal follows our current stability guarantees while > > > updating the guarantees for @PublicEvolving that we are already > providing > > > w/o having stated them. Moreover, this FLIP proposes some guidelines > for > > > determining the stability guarantees for an API object, how to evolve > > them > > > and some additional requirements for the return and parameter types of > > > methods. > > > > > > All in all, I hope that this proposal is more reflecting our current > > > understanding of stability guarantees than being controversial. One of > > the > > > outcomes of this FLIP should be to properly document these guarantees > so > > > that it is easily discoverable and understandable for our users. > > Moreover, > > > I hope that we can provide more stable APIs our users can rely and > build > > > upon. > > > > > > There will be a follow-up FLIP discussing the problem of how to make > sure > > > that APIs become stable over time. > > > > > > Looking forward to your feedback. > > > > > > [1] https://cwiki.apache.org/confluence/x/IJeqCw > > > [2] https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr > > > [3] https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr > > > > > > Cheers, > > > Till > > > > > >
