Thank you for your contribution! Please see my note below about the 10.2 versions. Also, you don't necessarily have to do this in this PR, but I'm thinking we should maybe deprecate the old versions that aren't supported anymore. Thoughts on that?

I think that it'll be better to do that in a new PR.
Just wondering, can we also remove all references to the older 10.0 and 10.1 versions (along with related minor versions), which cannot be launched at all now?

Thank you for your contribution! Please see my note below about the 10.2 versions. Also, you don't necessarily have to do this in this PR, but I'm thinking we should maybe deprecate the old versions that aren't supported anymore. Thoughts on that?

I think that it'll be better to do that in a new PR. Just wondering, can we also remove all references to the older 10.0 and 10.1 versions (along with related minor versions), which cannot be launched at all now?

aws-cdk/packages/@aws-cdk/aws-rds/lib/instance-engine.ts

Line 168 in 3f01c7d

public static readonly VER_10_0 = MariaDbEngineVersion.of('10.0', '10.0');

aws-cdk/packages/@aws-cdk/aws-rds/lib/instance-engine.ts

Line 209 in 3f01c7d

public static readonly VER_10_1 = MariaDbEngineVersion.of('10.1', '10.1');

I don't think we can just remove them but we should deprecate them.

Ah, now I see that we already have these deprecated in a bunch of places. I'm concerned that just removing them would break customer's code, which we don't want to do. I'm also concerned that there are no tests for the MariaDB versions. I know this seems like a very trivial change that shouldn't require tests but I'd like to see some sort of test to ensure we're generating the expected output of these new values. Doesn't need to be 100% coverage or anything.

@TheRealAmazonKendra I just added the deprecation notices for 10.2.

About the tests, is this what you had in mind?
https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-rds/test/sql-server/sql-server.instance-engine.test.ts#L20

Ah, now I see that we already have these deprecated in a bunch of places. I'm concerned that just removing them would break customer's code, which we don't want to do. I'm also concerned that there are no tests for the MariaDB versions. I know this seems like a very trivial change that shouldn't require tests but I'd like to see some sort of test to ensure we're generating the expected output of these new values. Doesn't need to be 100% coverage or anything.

@TheRealAmazonKendra I just added the deprecation notices for 10.2.

About the tests, is this what you had in mind? https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-rds/test/sql-server/sql-server.instance-engine.test.ts#L20

Yeah, something like that is good. I was also thinking of asking for tests on the change to deprecated, but I think that's overkill. We've tested that functionality in general and that would just be extra busy work. Thanks for also adding the EOL notes in there!

Ugh, the stupid trailing spaces issue. Just run yarn lint in this package real quick and it should fix that.

@TheRealAmazonKendra Tests added for all major versions and fixed the trailing spaces, should be ready now!

Just wondering, as this PR also contains the EOL notices maybe we should change the type to something else other than chore for making it show in the changelog?

@TheRealAmazonKendra Tests added for all major versions and fixed the trailing spaces, should be ready now!

Just wondering, as this PR also contains the EOL notices maybe we should change the type to something else other than chore for making it show in the changelog?

I think it's sufficient that it will show up in the doc string.

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 78258fd
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).