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.

Sign up for GitHub

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

Jump to bottom

refactor: update prefix logic in DA #152

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

refactor: update prefix logic in DA #152

wants to merge 21 commits into from
+30 −8

Conversation

arya-girish-k
Copy link

@arya-girish-k arya-girish-k commented Jan 13, 2025 •
edited
Loading

Description

Refactored the code logic for prefix variable by adding default value for prefix and marked as required in catalog manifest.Also allowed prefix to be "" (empty string) for advanced users.
#11959

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content

Add the default value for prefix variable and mark as required in catalog manifest. Also allowed prefix to be "" (empty string ).

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:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

For mergers

  • Use a conventional commit message to set the release level. Follow the guidelines.
  • Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
  • Use the Squash and merge option.

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

Please find the Test Reference
https://github.ibm.com/GoldenEye/issues/issues/11959#issuecomment-101688945

@arya-girish-k arya-girish-k self-assigned this Jan 15, 2025
iamar7
iamar7 previously approved these changes Jan 15, 2025
View reviewed changes
Copy link
Member

@iamar7 iamar7 left a comment

Choose a reason for hiding this comment

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

LGTM

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

/run pipeline

iamar7
iamar7 requested changes Feb 18, 2025
View reviewed changes
Copy link
Member

@iamar7 iamar7 left a comment

Choose a reason for hiding this comment

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

Can you please update the PR to use similar logic as in this one for prefix - terraform-ibm-modules/terraform-ibm-event-streams#373

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

Pre-commit failed due to this error.
image

To resolve this added the variable cbr_rules under project DA and removed cbr_rules from apps DA in ibm_catalog.json.

iamar7
iamar7 previously approved these changes Feb 20, 2025
View reviewed changes
Copy link
Member

@iamar7 iamar7 left a comment

Choose a reason for hiding this comment

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

LGTM

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

TestRunUpgradeAppSolution is failing due to the addition of a default value for the prefix variable. so skipping the upgrade test.
image

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

/run pipeline

ocofaigh
ocofaigh requested changes Mar 5, 2025
View reviewed changes
Copy link
Member

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

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

see comments

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k arya-girish-k requested a review from ocofaigh March 5, 2025 19:21
@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

Pipeline failed due to this error:
image

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k
Copy link
Author

Added Prefix in test,So re-running the pipeline.

@arya-girish-k
Copy link
Author

/run pipeline

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iamar7 iamar7 iamar7 left review comments

@akocbek akocbek Awaiting requested review from akocbek akocbek is a code owner

@shemau shemau Awaiting requested review from shemau shemau is a code owner

@ocofaigh ocofaigh Awaiting requested review from ocofaigh

Requested changes must be addressed to merge this pull request.

Assignees

@arya-girish-k arya-girish-k

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@arya-girish-k @iamar7 @ocofaigh