-
Notifications
You must be signed in to change notification settings - Fork 2
docs: added field description to enhance user-exp for S2S CRNs fields usage #124
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
base: main
Are you sure you want to change the base?
Conversation
/run pipeline |
/run pipeline |
/run pipeline |
@jor2 could you please review again? I modified the code from the previous commit |
/run pipeline |
/run pipeline |
/run pipeline |
/run pipeline |
/run pipeline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vbontempi Whats the difference between mq_s2s_policy_target_crn
and the new virtual input existing_mq_capacity_crn
you added? Seems like a duplicate to me?
ibm_catalog.json
Outdated
"virtual": true, | ||
"default_value": "__NULL__", | ||
"type": "string", | ||
"description": "The existing MQ capacity instance for MQ on Cloud Deployable architecture." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why would this say "Deployable architecture"? It doesn't have to come from a DA? oh wait I see what your trying to say... thinking about this....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current logic will:
Take the value of existing_mq_capacity_crn
virtual input, pass it to MQ DA, and then map it to the Ease4J input mq_s2s_policy_target_crn
. We are creating a whole extra layer of mapping here that is not needed right?
Could we not avoid this by doing this:
- Remove the virtual input
- Make
mq_s2s_policy_target_crn
required, but rename it with a display_nameexisting_mq_capacity_crn
and remove its default value and mark it as required. This would force the user to enter a value, but that value could be null if MQ was not in the mix.
EG:
{
"display_name": "existing_mq_capacity_crn",
"key": "mq_s2s_policy_target_crn",
"required": true,
"default_value": "__NOT_SET__",
"description": "MQ on Cloud capacity service instance CRN. Only required if deploying the Cloud automation for MQ on Cloud deployable architecture. Set to `null` is not deploying this."
},
/run pipeline |
exposed explicit existing_mq_capacity_crn input to enhance user experience (current mapping existing_mq_capacity_crn from mq_s2s_policy_target_crn is misleading)
Mapped explicit output of MQ DA capacity_crn to mq_s2s_policy_target_crn version input
Description
Release required?
x.x.X
)x.X.x
)X.x.x
)Release notes content
exposed virtual existing_mq_capacity_crn input of MQ DA
Mapped explicit output of MQ DA capacity_crn to mq_s2s_policy_target_crn version input
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