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

Add ibmcloud among supported platform for RHOAI install task #1364

Merged
merged 3 commits into from
Apr 10, 2024

Conversation

bdattoma
Copy link
Contributor

@bdattoma bdattoma commented Apr 9, 2024

No description provided.

@bdattoma bdattoma added verified This PR has been tested with Jenkins enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) labels Apr 9, 2024
@bdattoma bdattoma self-assigned this Apr 9, 2024
@bdattoma bdattoma added needs testing Needs to be tested in Jenkins and removed verified This PR has been tested with Jenkins labels Apr 9, 2024
Copy link
Contributor

github-actions bot commented Apr 9, 2024

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass %
445 0 0 445 100

@bdattoma bdattoma added verified This PR has been tested with Jenkins and removed needs testing Needs to be tested in Jenkins labels Apr 9, 2024
@bdattoma
Copy link
Contributor Author

bdattoma commented Apr 9, 2024

image

@@ -11,7 +11,7 @@ Library String
${cluster_type} selfmanaged
${image_url} ${EMPTY}
${RHODS_OSD_INSTALL_REPO} None
@{SUPPORTED_TEST_ENV} AWS AWS_DIS GCP PSI PSI_DIS ROSA
@{SUPPORTED_TEST_ENV} AWS AWS_DIS GCP PSI PSI_DIS ROSA IBM_Cloud
Copy link
Member

Choose a reason for hiding this comment

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

It's a nitpick, but could be possible to:

Suggested change
@{SUPPORTED_TEST_ENV} AWS AWS_DIS GCP PSI PSI_DIS ROSA IBM_Cloud
@{SUPPORTED_TEST_ENV} AWS AWS_DIS GCP PSI PSI_DIS ROSA IBM_Cloud

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

jstourac
jstourac previously approved these changes Apr 9, 2024
@@ -11,7 +11,7 @@ Library String
${cluster_type} selfmanaged
${image_url} ${EMPTY}
${RHODS_OSD_INSTALL_REPO} None
@{SUPPORTED_TEST_ENV} AWS AWS_DIS GCP PSI PSI_DIS ROSA
@{SUPPORTED_TEST_ENV} AWS AWS_DIS GCP PSI PSI_DIS ROSA IBM_Cloud
Copy link
Member

Choose a reason for hiding this comment

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

Hm, one more thing - I noticed now that we use uppercase for all the other variants. Wouldn't it be better to use IBM_CLOUD instead of IBM_Cloud? (same would have to apply also for the Jenkins code later on).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used Camel Case cos the others are all abbreviation.
I don't think it's a big deal, I can make it upper case but don't see much value in one of the other option

Copy link
Member

Choose a reason for hiding this comment

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

Upper case - easier to remember (don't have to think what part is uppercase and what part not); I don't insist. If nobody else has objections.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can use only IBM - 3 characters as most of the other platforms

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll make it upper case, cheap change.

Kobi I wanted to keep the original Cloud name like we do for the other providers

Copy link
Member

Choose a reason for hiding this comment

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

Don't know why, but i get FORTRAN vibes from this uppercase thing https://www.fortran90.org/src/best-practices.html

jstourac
jstourac previously approved these changes Apr 9, 2024
jiridanek
jiridanek previously approved these changes Apr 9, 2024
@bdattoma bdattoma dismissed stale reviews from jiridanek and jstourac via 7f50ac6 April 10, 2024 07:22
@@ -11,7 +11,7 @@
${cluster_type} selfmanaged
${image_url} ${EMPTY}
${RHODS_OSD_INSTALL_REPO} None
@{SUPPORTED_TEST_ENV} AWS AWS_DIS GCP PSI PSI_DIS ROSA
@{SUPPORTED_TEST_ENV} AWS AWS_DIS GCP PSI PSI_DIS ROSA IBM_CLOUD

Check notice

Code scanning / Robocop

Variable '{{ name }}' is assigned but not used Note

Variable '@{SUPPORTED_TEST_ENV}' is assigned but not used
@bdattoma bdattoma requested a review from jiridanek April 10, 2024 07:23
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.7% Duplication on New Code

See analysis details on SonarCloud

@bdattoma bdattoma merged commit b1de4c0 into red-hat-data-services:master Apr 10, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) verified This PR has been tested with Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants