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

feat(servicecatalog): Service Catalog is now in Developer Preview #19204

Merged
merged 18 commits into from
Mar 3, 2022

Conversation

arcrank
Copy link
Contributor

@arcrank arcrank commented Mar 1, 2022

Update some README documentation based on feedback.
Move library to developer preview with no anticipation of breaking changes.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

Aidan Crank and others added 2 commits March 1, 2022 14:29
…loper Preview

Update some of our readme language based on feedback.
Moving library to developer preview with expectation to move to stable soon.
@gitpod-io
Copy link

gitpod-io bot commented Mar 1, 2022

@github-actions github-actions bot added the @aws-cdk/aws-servicecatalog Related to AWS Service Catalog label Mar 1, 2022
@@ -37,7 +37,7 @@ enables organizations to create and manage catalogs of products for their end us
- [Constraints](#constraints)
- [Tag update constraint](#tag-update-constraint)
- [Notify on stack events](#notify-on-stack-events)
- [CloudFormation parameters constraint](#cloudformation-parameters-constraint)
- [CloudFormation template parameters constraint](#cloudformation-template-parameters-constraint)
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on CloudFormation template constraint instead?

Underlying SC terminology is that it is a Template Constraint and that Parameters are one of the examples a rule can apply to.

https://docs.aws.amazon.com/servicecatalog/latest/adminguide/reference-template_constraint_rules.html

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 think we've been moving towards more explicit naming. I'm not actually sure if assertions can apply to anything but the parameters in a meaningful way?

@arcrank
Copy link
Contributor Author

arcrank commented Mar 2, 2022

This is weird, build fails with:
Missing stability banner for developer-preview in README.md file (fixable)

I've just copied other PRs that only have the two changes to stability banner and package.json, not sure what is required now.

@skinny85
Copy link
Contributor

skinny85 commented Mar 2, 2022

Run yarn pkglint in the @aws-cdk/aws-servicecatalog package, should fix that for you 🙂.

@arcrank
Copy link
Contributor Author

arcrank commented Mar 2, 2022

Run yarn pkglint in the @aws-cdk/aws-servicecatalog package, should fix that for you 🙂.

Thanks, didn't realize a script was supposed to make those changes. Also linting is failing since no test changes, again I referenced other PRs but this might be a newer condition, should we have a test that asserts stability or something?

@skinny85
Copy link
Contributor

skinny85 commented Mar 2, 2022

Nah, I'll just add a label suppressing that rule.

@skinny85 skinny85 added the pr-linter/exempt-test The PR linter will not require test changes label Mar 2, 2022
Copy link
Contributor

@skinny85 skinny85 left a comment

Choose a reason for hiding this comment

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

Looks good, but please be consistent with making sure the line lengths don't get out of hand in the ReadMe file.

packages/@aws-cdk/aws-servicecatalog/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-servicecatalog/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-servicecatalog/README.md Outdated Show resolved Hide resolved
When launching a product, both the TagOptions associated with the product and the containing portfolio are made available.

At the moment, TagOptions can only be disabled in the console.
At the moment, TagOptions can only be deactivated in the console.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add this capability to the CDK L2? Seems very easy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could revisit this, the issue is that you cannot create a tagOption in a disabled state, only on update can it be deactivated. If we can have way to check for an on update in synth we could handle this.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can just underscore that in the documentation I think.

packages/@aws-cdk/aws-servicecatalog/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-servicecatalog/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-servicecatalog/README.md Outdated Show resolved Hide resolved
@mergify mergify bot dismissed skinny85’s stale review March 2, 2022 19:31

Pull request has been modified.

@arcrank
Copy link
Contributor Author

arcrank commented Mar 2, 2022

:| Small screen IDE auto wraps lines for me, I thought there used to be a linting rule that would fail? but I think they removed some of the stricter rules on the README since rosetta silently fails sometimes.

@mergify
Copy link
Contributor

mergify bot commented Mar 3, 2022

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

@skinny85
Copy link
Contributor

skinny85 commented Mar 3, 2022

@arcrank you might also want to think about doing this for the Service Catalog AppRegistry module (unless, of course, you think it's not mature enough to be called "Developer Preview").

@skinny85 skinny85 changed the title feat(servicecatalog): Service Catalog is now in developer preview feat(servicecatalog): Service Catalog is now in Developer Preview Mar 3, 2022
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: f9907cf
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify mergify bot merged commit 6dfc254 into aws:master Mar 3, 2022
@mergify
Copy link
Contributor

mergify bot commented Mar 3, 2022

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

@arcrank
Copy link
Contributor Author

arcrank commented Mar 3, 2022

@arcrank you might also want to think about doing this for the Service Catalog AppRegistry module (unless, of course, you think it's not mature enough to be called "Developer Preview").

Yup, already reached out to team, they expect some significant changes coming within < 6 months so holding off there. Thanks for being on top of things.

TheRealAmazonKendra pushed a commit to TheRealAmazonKendra/aws-cdk that referenced this pull request Mar 11, 2022
…s#19204)

Update some `README` documentation based on feedback.
Move library to developer preview with no anticipation of breaking changes.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-servicecatalog Related to AWS Service Catalog pr-linter/exempt-test The PR linter will not require test changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants