search

borkdude / api-diff

Print API diffs between library versions
59 stars 2 forks source link

Consider optionally returning result as edn #16

Open lread opened 2 years ago

lread commented 2 years ago

Proposal Optionally return api-diff result as a map.

Initial Ideas Command line would include optional :format which would default to :text but also allow for :edn.

The api-diff fn would no longer print and instead return the a result map. A separate report function would spit out result in requested format.

Next steps What would map contain?

borkdude commented 2 years ago

Yes. Prior art also available in clj-kondo and calva.

lread commented 2 years ago

Thanks! Here's an initial sketch of the map for review:

{:opts {<repeat opts used for diff here>}
 :findings [{:diff :removal
             :type :meta
             :data [:no-doc]
             :level :warning
             :v1 {:row x :col y :filename "z"}
             :v2 {:row a :col b :filename "c"}
             :ns my.ns.here
             :name var-name-here}
            {:diff :removal
             :type :now-private
             :level :error
             :v1 {:row x :col y :filename "z"}
             :v2 {:row a :col b :filename "c"}
             :ns my.ns.here :name var-name-here}
            {:diff :removal
             :type :arity
             :data 0
             :level :error
             :v1 {:row x :col y :filename "z"}
             :v2 {:row a :col b :filename "c"}
             ...}
            {:diff :removal
             :type :arity 
             :data 3
             :level :error
             :v1 {:row x :col y :filename "z"}
             :v2 {:row a :col b :filename "c"}
             ...}
            {:diff :removal
             :type :var
             :level :error
             :v1 {:row x :col y :filename "z"}
             <-- target var does not exist, so no :v2 -->
             ....}]}

There can be multiple findings for a single var.

I was thinking of also including full clj-kondo v1 and v2 var-definition data with each finding, but propose that we add that later if it seems fruitful.

borkdude commented 2 years ago

@lread I see that :data [:no-doc] is a vector and in other cases a number. I propose to just add bespoke keys for each type for extra data:

{:type :removed-arities
 :removed-arities [1 2 3]}

Note: the key doesn't have to have the same name as the type, we can add as many bespoke keys as necessary per type.

I also think that having a separate :diff and :type key isn't needed. Just one type can indicate what it is and the :level determines if this type indicates breakage.

lread commented 2 years ago

Hey, thanks for taking the time to review @borkdude, much appreciated!

I think your suggestion makes for a tighter, more succinct description. There is a certain beauty to the value for :type being the key for data (if any) for the type. (In other words, I really like the removal of :data).

When I sketched this out I was thinking of flexibility on how the data might be sliced and diced. But I think you are right, we (and others) can use :level to answer the most interesting questions.

I'll test this out against #14 but I think it should work very nicely, ex.

{:type :added-arities
 :added-arities [6]
 :level :info
 :v2 ...}

Would give us types with levels:

The cljdoc folks are currently discussing an API diffing feature, I'll review against their musings to see if I can find some future that will break our scheme.