standards-c-sharp icon indicating copy to clipboard operation
standards-c-sharp copied to clipboard

Handling Breaking Change in libraries

Open dicko2 opened this issue 3 years ago • 4 comments

Goals

We build libraries to share code. We OpenSource them to share with the community. We InnerSource them to share within our company.

The more people using the shared code the more we will get peer review, contribution, and grow.

Libraries that thrive and survive are ones that are consumer focused not producer focused. If you need to get a company wide mandate from upper management to use your library in the company, there is probably something wrong, I would ask yourself "Why aren't people using it already?" and go ask your potential consumers this question and get feedback and apply it.

Always remember your consumers are engineers. If you don't build what they need, they will simple ignore your code and write it themselves. And by the same token, if you make it hard to upgrade and get changes, they will stop using your code too, and your library will die off.

This is why breaking changes are an important topic.

Idea

There's two types of breaking change we want to talk about

  • Public Contract
  • Functional

First, Avoid breaking changes, Add new contracts with care

Always open an issue, discuss in a meeting and get pier review. Also don't forget to pay attention to English and fluency of the interface names, methods, etc Once Public contracts are in the wild they are hard to change without upsetting people

  1. Open issue
  2. Conduct meeting

Doing a breaking change: Changing or removing Public contracts

Should be planned well ahead.

Recommendations:

  • Obsolete attribute added 12 months ahead, and in the error give an example of which new method is a suitable replacement
  • Only done if absolutely needed, e.g. "I dont like the way the code looks" or "it's not clean" is generally a bad reason
  • Single Major version release with all breaking changes once a year, maybe align with framework updates if your library depends on net framework libraries

Supporting multiple versions?

Leave a branch open at the head of the previous major versions, or tag the release so people can easily patch an old version to fix bugs if they have not been able to upgrade. In general if you do a good job avoiding breaking changes this type of maintenance is not needed or greatly reduced.

How to avoid functional breaking changes?

There is two simple rules to follow that is the best way to ensure you don't have breaking functional changes, but there is no absolute way.

  1. Have good unit test coverage
  2. Never Change unit tests, only add them

You could also employee functional testing, depending on the library this may work as well as unit testing and you can apply the same run, i.e. only add test cases dont change them.

dicko2 avatar Jul 31 '21 02:07 dicko2

Another outcome we want from this is automation around

  • Detecting breaking contract change on a PR to block or warn (github action or similar automation)
  • Detecting changes to unit tests on a PR to block or warn (github action or similar automation)

dicko2 avatar Jul 31 '21 02:07 dicko2

https://docs.microsoft.com/en-us/dotnet/core/compatibility/ https://stackoverflow.com/questions/1456785/a-definitive-guide-to-api-breaking-changes-in-net

dicko2 avatar Sep 27 '21 14:09 dicko2

Simple tips for avoiding breaking changes:

Example Deleting/Removing Code

Don't remove ANYTHING public. Just don't. If you are adding new features, leave the old ones.

Example New features

If you are implementing new features, leave the old

In this example

// Example ordinal code

public class MyClass
{
public int DoStuff(int myInt)
{
// implementation
}
}

We want to add a new parameter to the 'DoStuff' method.


public class MyClass
{
public int DoStuff(int myInt, string myStr) // BREAKING CHANGE
{
// implementation
}
}

How should we handle this?


public class MyClass
{
public int DoStuff(int myInt) // original signature is maintained
{
 return DoStuff(myInt, "default string"); // call new overload with default
}
public int DoStuff(int myInt, string myStr) // new overload and move the implementation in here
{
// implementation
}
}

What about using parameter defaults.

Like this


public class MyClass
{
public int DoStuff(int myInt, string myStr = "default string") 
{
// implementation
}
}

I would avoid this, one it prevents muddying the definition of the function contract, and there is some caveats to option parameters explained below, while this is not a common scenario, the time you need to drop a dll onto a production sever in a hurry to save the day is the day this will bite you.

https://rules.sonarsource.com/csharp/RSPEC-2360

Example interface

There is two scenario's here.

  1. Your interface is NOT meant to be reimplemented outside the library
  2. Your interface is DESIGN TO be implemented is the Application that uses it

Scenario 1 is common due to testability, or you may prefer people use the Interface in your library so you can easily change the underlying implementations later with ease, in this scenario if its not the common use case that people will implement them, just follow the rules above for method change.

Scenario 2 a common example we have is a logging or metric implementation, in this case you need to be careful with new methods

// interface in my library
public interface IMetrics
{
 void SendMetrics(int value, string name);
}
// implementation in Consumer App
public class MyMetricSender : IMetrics
{
 pubic void SendMetrics(int value, string name);
{
// implementation
}
}

Now lets look at what happens when we add a new method

// interface in my library
public interface IMetrics
{
 void SendMetrics(int value, string name);
 void SendMetrics(int value, string name, Dictionary<string, string> tags);
}
// implementation in Consumer App
public class MyMetricSender : IMetrics // COMPILE ERROR, unimplemented methods
{
 pubic void SendMetrics(int value, string name);
{
// implementation
}
}

Since C# 8 you can deal with this when you use Default Methods in interfaces, I would recommend using these if possible, to avoid breaking people.

// interface in my library
public interface IMetrics
{
 void SendMetrics(int value, string name);
 void SendMetrics(int value, string name, Dictionary<string, string> tags) => Console.WriteLine("SendMetrics not implmetned");
}

dicko2 avatar Sep 27 '21 15:09 dicko2

Common Don'ts to watch out for

❌Don't move namespaces

❌Rename Parameters of methods (including changing its case) It breaks source compatibility when developers use named arguments.

❌Adding or removing the static keyword from a member

❌ Adding a constructor to a class that previously had no constructor without adding the parameterless constructor

❌Reducing the visibility of a member

dicko2 avatar Sep 27 '21 15:09 dicko2