search

Kotlin / binary-compatibility-validator

Public API management tool
Apache License 2.0
821 stars 59 forks source link

Stricter mode of API checking w.r.t. API file dump order #260

Open sandwwraith opened 3 months ago

sandwwraith commented 3 months ago

Currently, apiCheck task is tolerant to ordering changes in API file: if two lines with, let's say, top-level functions, are swapped, it won't cause apiCheck to fail. While generally, it is a desirable behavior, it has one major drawback. Since API file ordering is not stable and it can change (e.g. it was changed between 0.15 and 0.16 versions for Klib dumps), it is quite easy to get into the following situation:

  1. You have 0.15 dump committed
  2. You make a PR that updates BCV to 0.16. apiCheck passes; no other changes are required.
  3. You (perhaps several weeks later) add one function to a public API and do apiDump. Suddenly, not only has your new function been added to the API, but the whole diff has also become cluttered with lines reordering — change which is not related to your new PR per se.

To avoid this, we should either write BCV/dump version to the file (so updating plugin would force users to do new apiDump), or make a configuration option that would strictly compare files — so that people who want to avoid such situation can opt-in to order check.

JakeWharton commented 3 months ago

I would also advocate for this being the default and to be opt-out. I much prefer opting out of strictness than opting in, because it means that the presence of an opt-out is deliberate whereas the absence of an opt-in could merely be ignorance that the configuration even exists.