-
Notifications
You must be signed in to change notification settings - Fork 232
Remove default Artifacthub url override #3091
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
Conversation
|
I have verified fresh installation and upgrade with PAC PR , and it's working as expected. |
e74ba96 to
82f3ed5
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mbpavan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| } | ||
| defaultPacSettings := pacSettings.DefaultSettings() | ||
| defaultPacSettings := pacSettings.Settings{} | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have that code been merged into PAC Which handles the default hub ?
Can you share that PR as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
openshift-pipelines/pipelines-as-code#2353 yet to be merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pramodbindal It is not merged yet, PAC PR: openshift-pipelines/pipelines-as-code#2353
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then we should wait for that PR to merge first.
Once done we may need to upgrade the PAC dependency as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/hold
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pramodbindal We can merge this PR as this is in the main branch and we will have to cherry pick it in release-v0.77.x branch
We will update PAC dependency in the releasev0.77.x branch only
|
/kind bug |
82f3ed5 to
2a2c70a
Compare
|
@pratap0007 - Is there a need to revert the changes done for #3068 |
@anithapriyanatarajan I have included those change in this PR so there is no need to revert it |
|
/cherry-pick release-v0.78.x |
|
❌ Cherry-pick to The automatic cherry-pick to Output: Next steps:
|
|
/unhold |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
@anithapriyanatarajan @pratap0007 was the multi arch fixed in another PR ? |
Multiarch fixed with - #3106. |
|
Then, this needs a rebase @pratap0007 |
This patch includes the following changes: - Remove the default Hub catalog configuration from the PAC settings before PAC SyncConfig function call as this is already handled by the PAC SyncConfig function - Remove the default ArtifactHub URL override, which is managed by PAC. - Update the transform test to align with PAC changes, hub_url should not be fetchable, and PAC will now set the catalog type to tektonhub. Signed-off-by: Shiv Verma <[email protected]>
Signed-off-by: Shiv Verma <[email protected]>
228bf8f to
1a0443f
Compare
|
/lgtm |
This patch includes the following changes:
Changes
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
make test lintbefore submitting a PRSee the contribution guide for more details.
Release Notes