Sm addon migration #341
Sm addon migration #341
Conversation
ocofaigh
left a comment
ocofaigh
left a comment
There was a problem hiding this comment.
I left a few comments. We had a call with the docs team on Friday, and all of these types of PRs need to be reviewed by the docs team since they are adding / updating content that will be shown in the tile (descriptions, features etc etc)
| }, | ||
| { | ||
| "name": "deploy-arch-ibm-observability", | ||
| "description": "Enable to provisions and configures IBM Cloud Monitoring, Activity Tracker, and Log Analysis services for analysing events generated from the Events Notification instance.", |
There was a problem hiding this comment.
Description need to be fixed. It is inaccurate and referencing deprecated service. See the update I made in EN DA. But also docs team should review final proposal.
There was a problem hiding this comment.
|
I have left all the documentation comments, as I believe each one of these + any other docs related comment can be addressed in a separate PR |
|
/run pipeline |
smguilia
left a comment
smguilia
left a comment
There was a problem hiding this comment.
There are some common issues to be on the lookout for in future PRS:
-
Use the legally approved service names. Verify that they're on this list: https://github.ibm.com/cloud-doc-build/markdown/blob/master/cloudoekeyrefs.yml
-
Be sure you're not dropping words like "The" or "An / a"
-
Capitalization of Cloud automation for _____ should be consistent.
-
For the descriptions in the catalog - I would focus more on the benefits of the things deploying in the descriptions rather than just saying that they're integrated or provisioned. Let me know if this one doesn't make sense. I put in several comments but they might not be 100% technically accurate.
…/terraform-ibm-secrets-manager into sm-addon-migration
Co-authored-by: Shawna Hinger <[email protected]>
Co-authored-by: Shawna Hinger <[email protected]>
Co-authored-by: Shawna Hinger <[email protected]>
Co-authored-by: Shawna Hinger <[email protected]>
Co-authored-by: Shawna Hinger <[email protected]>
Co-authored-by: Shawna Hinger <[email protected]>
Co-authored-by: Shawna Hinger <[email protected]>
Co-authored-by: Shawna Hinger <[email protected]>
|
/run pipeline |
|
@smguilia thanks for the review. |
ocofaigh
left a comment
ocofaigh
left a comment
There was a problem hiding this comment.
Update the variable description for secret_groups to mention about prefix being used in access group name (for both variations).
Also as Steve pointed out yesterday we need to include the information about the default "General" secret group that gets created in more places. Descriptions, Features, Diagram description etc
| }, | ||
| { | ||
| "name": "deploy-arch-ibm-observability", | ||
| "description": "Enable to provisions and configures IBM Cloud Monitoring, Activity Tracker, and Log Analysis services for analysing events generated from the Events Notification instance.", |
There was a problem hiding this comment.
secret group is already part of features and its in the diagram. Not sure what else need to be added in the Description and diagram description, especially around "General" because user can change that value. |
…/terraform-ibm-secrets-manager into sm-addon-migration
|
/run pipeline |
|
🎉 This PR is included in version 2.4.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Description
Release required?
x.x.X)x.X.x)X.x.x)Release notes content
Run the pipeline
If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.
Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:
Checklist for reviewers
For mergers