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

feat: consistency with other ICD modules #520

Merged
merged 27 commits into from
Mar 10, 2025
Merged

feat: consistency with other ICD modules #520

merged 27 commits into from
Mar 10, 2025

Conversation

Aayush-Abhyarthi
Copy link
Contributor

@Aayush-Abhyarthi Aayush-Abhyarthi commented Feb 10, 2025 •
edited by shemau
Loading

Description

https://github.ibm.com/GoldenEye/issues/issues/11676

Release required?

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

Address consistency issues:

Issues addressed that require code changes to upgrade:

  • Input instance_name renamed to name
  • Input endpoints renamed to service_endpoints

Issues addressed in the deployable architecture

  • Removed cbr_rule_ids, adminuser and certificate_base64 outputs.
  • Added input existing_redis_instance_crn, admin_pass_secret_manager_secret_group, use_existing_admin_pass_secret_manager_secret_group and admin_pass_secret_manager_secret_name inputs.
  • Support for passing an existing instance

Issues addressed that do not require code changes to upgrade:

  • Adds missing examples to cover restoring from backup
  • Balanced tests to improve coverage with fewer resources

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.

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi Aayush-Abhyarthi changed the title initial commit feat: added missing examples and plugged test gaps Feb 15, 2025
@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi Aayush-Abhyarthi marked this pull request as ready for review February 19, 2025 20:18
@shemau
Copy link
Contributor

shemau commented Feb 20, 2025

I recently renamed the advanced example, complete example to be more consistent with other ICD repos. This is causing the resolve conflicts above.

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

shemau
shemau requested changes Feb 24, 2025
View reviewed changes
@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi Aayush-Abhyarthi requested a review from shemau March 3, 2025 08:39
@shemau shemau changed the title feat: added missing examples and plugged test gaps feat: consistency with other ICD modules Mar 3, 2025
shemau
shemau previously approved these changes Mar 3, 2025
View reviewed changes
Copy link
Contributor

@shemau shemau left a comment

Choose a reason for hiding this comment

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

LGTM

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

@ocofaigh ocofaigh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I left a comment. Also why did we bump to terraform >= 1.9.0 but we did not roll out any terraform 1.9 features (such as cross variable validation)? I would suggest those changes come together in their own PR.

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@Aayush-Abhyarthi
Copy link
Contributor Author

/run pipeline

@shemau shemau self-requested a review March 10, 2025 10:48
shemau
shemau approved these changes Mar 10, 2025
View reviewed changes
Copy link
Contributor

@shemau shemau left a comment

Choose a reason for hiding this comment

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

LGTM

@ocofaigh ocofaigh merged commit 1f22f28 into main Mar 10, 2025
2 checks passed
@ocofaigh ocofaigh deleted the test-coverage branch March 10, 2025 11:03
@terraform-ibm-modules-ops
Copy link
Contributor

🎉 This PR is included in version 1.18.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

@shemau shemau shemau approved these changes

@ocofaigh ocofaigh ocofaigh approved these changes

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

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Aayush-Abhyarthi @shemau @terraform-ibm-modules-ops @ocofaigh