Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

Please can you use semver? #178

Closed
marcosscriven opened this issue Dec 23, 2019 · 4 comments
Closed

Please can you use semver? #178

marcosscriven opened this issue Dec 23, 2019 · 4 comments

Comments

@marcosscriven
Copy link

marcosscriven commented Dec 23, 2019

We have a process to automatically update any dependencies to the latest minor or patch version, on the assumption that won't contain breaking changes, but will bring in security fixes and additional backward-compatible features.

4.9.0 has broken this as LDUser.getValueForEvaluation now returns an LDValue.

So this is just a request/datapoint on using semver, and arguing to either have labelled this 5.0.0, or added a new method to LDUser, and deprecate the other.

@eli-darkly
Copy link
Contributor

That method was meant to be package-private - it is an implementation detail - and changing package-private methods is not considered a breaking change. However, it was for some reason declared with protected scope instead. That was certainly a mistake; but in normal usage of the SDK no one would be subclassing User, so application code would never be referencing this method. Could you clarify whether this was in fact a breaking change for your code?

To reiterate, this was a mistake on our part; our policy, as indicated in all of our changelogs, is to adhere to semver even in the case of a method that had been exposed unintentionally. But since we cannot unrelease the 4.9.0 release and, as mentioned, it is hard to imagine this actually affecting application code, I think the best course would be to simply acknowledge the error and move on.

@bwoskow-ld
Copy link
Member

Hi @marcosscriven,

@eli-darkly beat me to it in responding to your message. I wanted to add a couple items:

  1. I agree with Eli that the best course of action at this point is to acknowledge the error and move on. 4.9.0 was released over two months ago and as such, reverting the breaking change would in turn be a breaking change for anyone already using this version or a newer version.
  2. On the topic of generally attempting to follow semantic versioning guidelines -- you can read our SDK versioning policy to learn more about how we version our SDKs. This document helps define how we handle some of the ambiguities in the semantic versioning guidelines.

Cheers,
Ben

@marcosscriven
Copy link
Author

marcosscriven commented Dec 23, 2019

Hi @bwoskow-ld and @eli-darkly

Thanks for the prompt and helpful responses. This did indeed break a single line of our code, but only at compile time so not a big issue.

It did also happen to break a few tests as the API key in config is now null checked.

Both easily fixed - it just means busy work for an otherwise fully automated process.

@bwoskow-ld
Copy link
Member

I'm glad it was easy to work around. Our apologies for the slight inconvenience!

LaunchDarklyCI pushed a commit that referenced this issue Jan 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants