Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add a notice for AWS CDK v1 going into maintenance mode soon #9

Merged
merged 5 commits into from
Apr 12, 2022

Conversation

madeline-k
Copy link
Contributor

@madeline-k madeline-k commented Apr 8, 2022

I tried to keep the notice short, and added more detail in the linked issue: aws/aws-cdk#19836

Fixes https://github.com/cdklabs/cdk-ops/issues/1934

@madeline-k madeline-k changed the title Add a notice for AWS CDK v1 going into maintenance mode soon chore: add a notice for AWS CDK v1 going into maintenance mode soon Apr 8, 2022
@madeline-k madeline-k requested review from otaviomacedo and a team April 8, 2022 20:47
@otaviomacedo
Copy link
Contributor

The version is valid, but it's not passing the test 🤷‍♂️. You can try and replace it with <2.0.0

{
"title": "AWS CDK v1 entering maintenance mode soon",
"issueNumber": 19836,
"overview": "AWS CDK v1 is entering maintenance mode on June 1, 2022. Migrate to AWS CDK v2 to continue to get the latest features and fixes!",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nit, but maybe we should provide a link to the migration documentation at the end of this message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a nit. The migration documentation is essential for people to be able to take action from this notice and we want it to be easy to find. I included it in the linked github issue. At the bottom of each notice, the CLI prints out "More information at: https://github.com/aws/aws-cdk/issues/<#>". Because of this, I chose to not include a link to the migration guide directly here, to not overwhelm the CLI output with a bunch of links. Do you think it will be easy enough to find that way? Or should we include it here as well?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should add any unique information from the README into the "Migrate to v2" topic in the Developer Guide, and link directly to that topic from the CLI?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which README?

We have two options here (let me know if there are more):

  1. (Current implementation) The CLI notice will have a single link to AWS CDK v1 will enter Maintenance Mode after June 1, 2022 aws/aws-cdk#19836. This issue has a link to the "Migrating to AWS CDK v2" topic in the Developer Guide.
  2. The CLI notice will link to both AWS CDK v1 will enter Maintenance Mode after June 1, 2022 aws/aws-cdk#19836 and https://docs.aws.amazon.com/cdk/v2/guide/migrating-v2.html

Option 1 is a cleaner CLI output, but requires 2 clicks to get to the "Migrating to AWS CDK v2" page.

I'm still leaning toward option 1 as implemented. But I it's important we make this documentation easy to find for people. So I'm open to option 2 as well.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's a clear path to the migration documentation, and I think there is from that issue, I would be inclined to agree with the current implementation. I didn't know that the "more information at..." line was included so I think this is good.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, the current implementation is the way to go.

@madeline-k madeline-k merged commit a0dd27a into main Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants