openai/openai-dotnet

Consider enabling APICompat

Closed this issue · 4 comments

ericstj commented

It may be beneficial to run APICompat on this library to ensure that it doesn't make breaking changes.

I noticed between 2.2.0 and 2.3.0 there was one public breaking change -- #507 (comment)

I also found that internal breaking changes caused Azure.OpenAI to break -- Azure/azure-sdk-for-net#51731

These could be caught by running APICompat against internals -- however that might be much more noisy since it looks like there are many internal breaking changes. Ideally such libraries that are built seperately like this wouldn't rely on InternalsVisibleTo. It seems ripe for this sort of fragility.
https://gist.github.com/ericstj/663a0a5343afd253412a24223878240e

@jsquire

joseharriaga commented

Thank you for reaching out, @ericstj !

Regarding public breaking changes, we do run APICompat:
🔗 https://github.com/openai/openai-dotnet/blob/main/scripts/Test-ApiCompatibility.ps1
Although I did notice that it flags changes in Experimental APIs as breaking. Do you know if there is a way to filter those out without having to create individual exceptions for each?

Regarding internal breaking changes, us together with the Azure OpenAI team are actively working on a solution that will help Azure OpenAI move away from relying on InternalsVisibleTo in the near future precisely to address this issue (among a few others). 🙂

  • ❤️2
ericstj commented

Although I did notice that it flags changes in Experimental APIs as breaking. Do you know if there is a way to filter those out without having to create individual exceptions for each?

That's a good feature suggestion. I opened dotnet/sdk#50090

  • ❤️1
joseharriaga commented

Nice! Thank you, @ericstj!

ericstj commented

We don't excuse experimental in runtime, since we typically still want to know about breaking changes in public experimental API, and we're happy to just suppress those as part of the "documentation in source" that the breaking changes are intentional.

Given the goal is to move away from IVT, it probably doesn't make sense to push on better tracking those (such as distinguishing which internals can/cannot change). I've seen this problem come up before --> dotnet/machinelearning@f920262 but most of the time folks find that trying to do this is not worth the complexity and instead resort to pub-ternals where they tell folks not use publics with documentation, or hiding from design time.

I'll close this issue since it seems like this is already in use.