open-constructs / aws-cdk-library

Community-Driven CDK Construct Library
Apache License 2.0
65 stars 15 forks source link

Handling version updates #36

Open dbartholomae opened 5 months ago

dbartholomae commented 5 months ago

dependencies

Currently we don't have any, and we might never add some to avoid all the pain that would bring, so I'll leave them out of the current discussion.

devDependencies

These should be always updatable and we should be able to fully automate this.

peerDependencies

We currently rely on aws-cdk-lib and constructs and most likely will forever. So for any discussion on this topic, we can always concretely think about these two. There was also already a discussion on this in #12

We have mainly two competing interests:

Overall, any kind of change can be a breaking change to someone, so for me how we handle this isn't only about versioning, but about what we communicate about our versioning.

So far, I roughly see the following options

  1. Don't upgrade. This is out I would say as we need access to L1 updates to be able to write L2s.
  2. Manually investigate for each version change of a dependency the chances of this breaking things for users, and choose our version appropriately. This basically means doing the version policy work for the aws-cdk team and from my point of view also is out due to the high effort.
  3. Mark each peerDependency upgrade as a breaking change, but still update regularly. This would lead to high version numbers (which I personally don't mind but can create a weird impression with users who are used to the versioning scheme of aws-cdk-lib), and it could make it hard for users to identify actually really breaking changes.
  4. Follow the same version scheme as aws-cdk-lib and constructs: If they consider a change a minor change, we also include it as a minor change, etc. - this is my personal favorite, as the aws-cdk team so far was very mindful about breaking changes outside of alpha-modules, and I don't see a high risk of someone needing to upgrade the dependency on our lib without being able to upgrade their dependency on aws-cdk-lib and constructs as well.
  5. Start with a version range that indicates less stability first (e.g. 0.x.x) and communicate that these might have breaking changes all the time, and progress to another versioning policy once we have learnt more about the library, and so many people depend on it that the versioning policy actually starts to matter.
  6. Don't care about changes coming from version updates and add them without affecting the version of our library. I think that this might also work in our case as the aws-cdk is very cautious about breaking changes on their side.

Regardless of the version policy we decide for, I would be in favour of documenting it in the readme so that our users are aware of it.

mrgrain commented 5 months ago

Random collection of my thoughts:

dependencies

Currently we don't have any, and we might never add some to avoid all the pain that would bring, so I'll leave them out of the current discussion.

It will be bundled dependencies and we will mostly like need similar ones to aws-cdk-lib, e.g. yaml or case. See: https://github.com/aws/aws-cdk/blob/af81264613cec651d4ffb69db187ddaac64048df/packages/aws-cdk-lib/package.json#L108-L120

  1. Don't upgrade. This is out I would say as we need access to L1 updates to be able to write L2s.

I don't think this is correct. We can absolutely take a peerDependency on say aws-cdk-lib@^2.50.0 and use aws-cdkl-lib@latest as a local devDependency for the development. Note the ^ on the peerDependency constraint. That means consumers of our package can bring whatever version of aws-cdk-lib they like, as long as it's newer than 2.50.0. Because we would use latest as the version to develop against, we can still introduce constructs based on new L1s or even L2.

For this to work properly, we would need to setup some additional stuff though. In the CI we will need to run our tests against the lowest and the highest version of aws-cdk-lib. On GitHub that can be done quite easily with a matrix build. In our code base we will need new helpers that deal with versioning. I reckon these two would catch most of the cases:

A helper to declare a minimal required version for a certain feature:

/**
* Imports a service module from `aws-cdk-lib` while taking into account a minimal version requirement.
* If the minimal version requirement is not met, it will throw with an helpful error message.
* 
* @param service The service module to import.
* @param minVersion The minimal version of the module that is required for usage. Defaults to the minimal supported version of the whole package.
* @return The imported service module.
*/
function requireCdk(service: string, minVersion: string = '2.50.0') {
  const installedVersion = getInstalledCdkVersion(); // this should be a global singleton
  if (new Semver(minVersion).lt(installedVersion) {
    throw `This feature requires aws-cdk-lilb to be >=${minVersion}, but found ${installedVersion}. Please update aws-cdk-lib to use this feature.`;
  }
  return require(`aws-cdk-lib/${service}`);
}

// Usage
const s3 = requireCdk('s3', '2.70.0');

A helper to make testing multiple aws-cdk-libs easier:

test.needsCdk('2.70.0')("testing my feature", () => {
  // this test would run if the installed version is >= 2.70.0 but will be skipped if not
});

It's still not a perfect system though. And people might not appreciate the shift to runtime validation, although that is technically what peerDependencies are supposed to do. Also makes documentation harder. I'm not even sure I would suggest doing this. But wanted to call out that this is absolutely possible.

  1. Manually investigate for each version change of a dependency the chances of this breaking things for users, and choose our version appropriately. This basically means doing the version policy work for the aws-cdk team and from my point of view also is out due to the high effort.
  2. Mark each peerDependency upgrade as a breaking change, but still update regularly. This would lead to high version numbers (which I personally don't mind but can create a weird impression with users who are used to the versioning scheme of aws-cdk-lib), and it could make it hard for users to identify actually really breaking changes.

This relates to 2 & 3 together. I'll try to be clear as possible. From a purist semver point of view, narrowing the peerDependency constraint is the breaking change. And yes it is also actually really breaking. And that is simply because it will not be enough to update @open-constructs/aws-cdk anymore, but you also have to make another manual intervention.

Like most breaking changes it will definitely not affect all users. You can argue about how many users it would affect, but from the semver purist point of view it doesn't matter. It might affect someone, and that's why it's breaking. That means it also doesn't matter if the underlying change in aws-cdk-lib has a "breaking change" or not (it really shouldn't anyway).

I can appreciate that people have a different view on this. I get it. It feels like an annoying thing. At the end of the day, to me the trade-off is:

There is a lot of gray in between these two. I still believe the best balance is a regular, predictable update cadence. But I'll definitely happy with either choice.

  1. Follow the same version scheme as aws-cdk-lib and constructs: If they consider a change a minor change, we also include it as a minor change, etc. - this is my personal favorite, as the aws-cdk team so far was very mindful about breaking changes outside of alpha-modules, and I don't see a high risk of someone needing to upgrade the dependency on our lib without being able to upgrade their dependency on aws-cdk-lib and constructs as well.

We will need to think about feature flags. aws-cdk-lib only works this way because breaking changes are feature flagged. (How) Can we introduce code that depends on a feature flag being enabled?

  1. Start with a version range that indicates less stability first (e.g. 0.x.x) and communicate that these might have breaking changes all the time, and progress to another versioning policy once we have learnt more about the library, and so many people depend on it that the versioning policy actually starts to matter.

This is seems like a good thing to do anyway.

  1. Don't care about changes coming from version updates and add them without affecting the version of our library. I think that this might also work in our case as the aws-cdk is very cautious about breaking changes on their side.

Same as 4) really.

Regardless of the version policy we decide for, I would be in favour of documenting it in the readme so that our users are aware of it.

100%

WinterYukky commented 5 months ago

We will need to think about feature flags. aws-cdk-lib only works this way because breaking changes are feature flagged. (How) Can we introduce code that depends on a feature flag being enabled?

I think this message is out of scope this topic. This message means to add breaking changes for this repository source code and release it. This also important. However, this idea focuses to consider as breaking change or not to update aws-cdk-lib.

The important things are this repository is extends of aws-cdk-lib and aws-cdk-lib is not simple peerDepnedency like a other construct library. If a customer wants new feature or bug fix, the customer update aws-cdk-lib. If the customer doesn't wanna any update not so. These most customer take same actions with this package. I think it is unlikely that you will update just this package and not update aws-cdk-lib.

WinterYukky commented 5 months ago

I agree with part of this opinions and these opinions are not exclusive opinions and you can select multiple opinions. I agree ideas 4 and 5.

mrgrain commented 5 months ago

Thought a bit more about this. In my mind we have two realistic options:

Regardless, we should stay in major version zero for some time to find our groove.

Why am I not keen on some loose connection?

I just think it creates to much ambiguity. If we update our peerDep on aws-cdk-lib but don't align with upstream versions, then for every MINOR update, users will have to manually check what that means for aws-cdk-lib.

Recommendation: Option A

From what I'm hearing from other contributors, I think Option A is the way to go.

badmintoncryer commented 3 months ago

Is there any progress? I want to create a PR based on aws-cdk-lib newer than version 2.120.0. I am eagerly waiting for this issue to be resolved in any way possible!

mrgrain commented 3 months ago

Is there any progress? I want to create a PR based on aws-cdk-lib newer than version 2.120.0. I am eagerly waiting for this issue to be resolved in any way possible!

I think you can just open your PR. We seem to have consensus that this library should stay in major version zero for a bit, which gives us leeway for changes.