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

fix: corrected deprecated usage of ModuleLister.moduleAdded (#732) #740

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adietish
Copy link
Contributor

(partially) fixes #732

@adietish adietish self-assigned this Mar 12, 2024
@openshift-ci openshift-ci bot requested a review from sbouchet March 12, 2024 15:01
Copy link

openshift-ci bot commented Mar 12, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from adietish. For more information see the Kubernetes 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

@adietish adietish changed the title corrected deprecated usage of ModuleLister.moduleAdded (#732) fix: corrected deprecated usage of ModuleLister.moduleAdded (#732) Mar 12, 2024
@adietish adietish force-pushed the issue-732 branch 2 times, most recently from 8120951 to f6109f5 Compare March 12, 2024 15:27
Copy link

Copy link

openshift-ci bot commented Mar 12, 2024

@adietish: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-openshift f6109f5 link true /test e2e-openshift

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@adietish
Copy link
Contributor Author

ModuleListener#modulesAdded was added in IC-2022.3 and this PR thus requires us to bump the minimal version from (currently) IC-2022.1 to IC-2022.3

@sbouchet
Copy link
Collaborator

ModuleListener#modulesAdded was added in IC-2022.3 and this PR thus requires us to bump the minimal version from (currently) IC-2022.1 to IC-2022.3

I think we'll bump minimal version when 2024.1 will be out .

Copy link

@adietish
Copy link
Contributor Author

ModuleListener#modulesAdded was added in IC-2022.3 and this PR thus requires us to bump the minimal version from (currently) IC-2022.1 to IC-2022.3

I think we'll bump minimal version when 2024.1 will be out .

I checked it and found out that I was wrong: the API required for this change only exists in IC-2023.1 (not present in IC-2022.3).

@sbouchet
Copy link
Collaborator

@adietish this is still a warning on current 1.9.0 release. can you update this fix with latest main branch ?

@adietish
Copy link
Contributor Author

@sbouchet: we're currently still at IC-2022.3 as minimum supported version. For this fix to be applicable we need IC-2023.1 (see #740 (comment))

Copy link

@mohitsuman
Copy link
Collaborator

@adietish what is the update on this PR ?

@adietish
Copy link
Contributor Author

adietish commented Sep 18, 2024

@mohitsuman, @sbouchet: we're still at 2022.3: Do we want to bump so that we can include this PR?

@mohitsuman
Copy link
Collaborator

@adietish Are there any challenges in bumping ?

@mohitsuman
Copy link
Collaborator

And should we move this PR our of Draft state ?

@sbouchet
Copy link
Collaborator

@mohitsuman @adietish : I propose to merge this PR after #889, bumping minimal version to 2023.1 .

@mohitsuman
Copy link
Collaborator

@sbouchet PR #889 can be merged when ?

@sbouchet
Copy link
Collaborator

sbouchet commented Oct 1, 2024

@sbouchet PR #889 can be merged when ?

almost, there is still Integration UI test failures that @martinszuc is looking at

@adietish
Copy link
Contributor Author

adietish commented Oct 2, 2024

@sbouchet rebased it.

Copy link

sonarqubecloud bot commented Oct 2, 2024

@sbouchet
Copy link
Collaborator

sbouchet commented Oct 4, 2024

as expected, minimal version check (22.3) failed. let's wait for minimal version bump before merging this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix usage of deprecated APIs
3 participants