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

First release cleanup #11

Merged
merged 46 commits into from
Feb 15, 2024
Merged

First release cleanup #11

merged 46 commits into from
Feb 15, 2024

Conversation

vbontempi
Copy link
Member

@vbontempi vbontempi commented Feb 9, 2024

Description

First part of the cleanup task from #8

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content
  • Scaled down complete test cluster
  • reviewed WebSphere Liberty naming
  • Removed commented code
  • Fixed kubeconfig path for datasource
  • Removed default region for extension
  • Prefixed sample app URL with https:// to make it clickable when deployed through DA
  • Made resource groups names prefixed with random prefix with SLZ test override json

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.

vbontempi and others added 30 commits January 31, 2024 18:40
Co-authored-by: Rajat Agrawal <[email protected]>
@vbontempi
Copy link
Member Author

@SirSpidey I think I addressed all your comments and questions and pushed the changes

@vbontempi
Copy link
Member Author

/run pipeline

SirSpidey
SirSpidey previously approved these changes Feb 12, 2024
Copy link
Contributor

@SirSpidey SirSpidey left a comment

Choose a reason for hiding this comment

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

I'm fine with leaving the kubeconfig readme, I just don't understand what it does and why we would leave a link to an internal issue.

@vbontempi
Copy link
Member Author

I'm fine with leaving the kubeconfig readme, I just don't understand what it does and why we would leave a link to an internal issue.

right, the internal link is to be removed, done

@vbontempi
Copy link
Member Author

/run pipeline

@rajatagarwal-ibm
Copy link
Member

rajatagarwal-ibm commented Feb 12, 2024

Here scripts/confirm-wsloperator-operational.sh:

  • -o wide flag doesn't work in the following command:
kubectl describe Subscription ibm-websphere-liberty -n "${namespace}" -o wide

Result of running the above command:

10135 ± kubectl describe Subscription ibm-websphere-liberty -n openshift-operators -o wide                                                                                                                                       
error: unknown shorthand flag: 'o' in -o
See 'kubectl describe --help' for usage.

@rajatagarwal-ibm
Copy link
Member

rajatagarwal-ibm commented Feb 12, 2024

Need to update readme for the examples/complete/README.md.

  • Not creating 3 subnets anymore.
  • Not creating 3 worker pools anymore.

@rajatagarwal-ibm
Copy link
Member

Should update examples/complete/main.tf

@rajatagarwal-ibm
Copy link
Member

The title is wrong in this readme extensions/landing-zone/README.md

@vbontempi vbontempi requested a review from SirSpidey February 13, 2024 15:02
@vbontempi
Copy link
Member Author

Need to update readme for the examples/complete/README.md.

* Not creating 3 subnets anymore.

* Not creating 3 worker pools anymore.

2 workers are the min for OCP. Reduced to 2.

All the other comments are addressed and pushed, please review

@vbontempi
Copy link
Member Author

/run pipeline

Copy link
Contributor

@SirSpidey SirSpidey left a comment

Choose a reason for hiding this comment

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

a few more small suggestions

examples/complete/README.md Outdated Show resolved Hide resolved
examples/complete/README.md Outdated Show resolved Hide resolved
examples/complete/README.md Outdated Show resolved Hide resolved
examples/complete/README.md Outdated Show resolved Hide resolved
extensions/landing-zone/variables.tf Outdated Show resolved Hide resolved
@vbontempi
Copy link
Member Author

a few more small suggestions

thank you for the suggestions, committed all of them, could you please review?

@vbontempi
Copy link
Member Author

/run pipeline

sync description with readme
Copy link
Contributor

@SirSpidey SirSpidey left a comment

Choose a reason for hiding this comment

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

also updated the settings.yml description in this PR.

@rajatagarwal-ibm
Copy link
Member

/run pipeline

@vburckhardt vburckhardt merged commit b820f8c into main Feb 15, 2024
2 checks passed
@vburckhardt vburckhardt deleted the fix_first_release branch February 15, 2024 11:27
@ocofaigh
Copy link
Member

@vburckhardt @vbontempi @rajatagarwal-ibm The PR was not merged with a commit message that will generate a release. If you need a release like you indicated in the PR description, a new PR needs to be created and merged with correct prefix

@terraform-ibm-modules-ops
Copy link
Contributor

🎉 This PR is included in version 1.0.1 🎉

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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants