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

modified rhtap installation to accept the installation options generated by pict #366

Merged
merged 1 commit into from
Dec 31, 2024

Conversation

xinredhat
Copy link
Member

@xinredhat xinredhat commented Dec 27, 2024

modified rhtap installation to accept the installation options generated by pict

@xinredhat xinredhat force-pushed the pict_installation branch 2 times, most recently from c10e6c1 to 0685437 Compare December 27, 2024 10:31
@xinredhat xinredhat changed the title [WIP]modified rhtap installation to accept the installation options generated by pict modified rhtap installation to accept the installation options generated by pict Dec 27, 2024
@xinredhat
Copy link
Member Author

/retest

@xinredhat
Copy link
Member Author

xinredhat commented Dec 30, 2024

The both sub-pipelineruns of pr-e2e-tests-5s68f Succeeded, I think we can merge this PR. @prietyc123 WDYT?

e2e-4.17-hkfkh
e2e-4.16-x6w6g

- name: tpa_config
type: string
description: "The TPA option for rhtap-cli installation. Valid values are 'new' and 'hosted'."
- name: registry_config
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we consider any default value? Same question for other params as well

Copy link
Member Author

Choose a reason for hiding this comment

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

I would like to add a default.pict in repository to specify the default values. For example PR#203 So the modification of install.sh won't break the expected result. If we need to consider any default value in task or pipline yaml files in the future we can set default values.

@prietyc123
Copy link
Contributor

The both sub-pipelineruns of pr-e2e-tests-5s68f Succeeded, I think we can merge this PR. @prietyc123 WDYT?

e2e-4.17-hkfkh e2e-4.16-x6w6g

makes sense merging it. However I have added few small comments. Could you please take a look

@prietyc123
Copy link
Contributor

/approve

@prietyc123
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Dec 30, 2024
@prietyc123
Copy link
Contributor

/approve

Copy link

openshift-ci bot commented Dec 30, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: prietyc123, xinredhat
Once this PR has been reviewed and has the lgtm label, please assign otaviof for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@xinredhat xinredhat merged commit 0ab90e1 into redhat-appstudio:main Dec 31, 2024
10 of 11 checks passed
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.

2 participants