Skip to content

Align set with k/k apimachinery#294

Open
skitt wants to merge 2 commits into
kubernetes:masterfrom
skitt:align-apimachinery-set
Open

Align set with k/k apimachinery#294
skitt wants to merge 2 commits into
kubernetes:masterfrom
skitt:align-apimachinery-set

Conversation

@skitt

@skitt skitt commented Nov 23, 2023

Copy link
Copy Markdown
Member

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This aligns the features of generic Set with the version in apimachinery. The main change is that sets can be instantiated with any comparable type, not only ordered types; as a result, SortedList() can no longer be implemented as a method, and is replaced by the List() function to match apimachinery.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

sets.Set accepts any comparable type; the sets.SortedList method is replaced with a sets.List function, to match the apimachinery implementation, and to allow unordered sets to be converted to ordered slices.

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 23, 2023
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 23, 2023
logicalhan
logicalhan approved these changes Nov 23, 2023
View reviewed changes

@logicalhan logicalhan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 23, 2023
@skitt

skitt commented Nov 23, 2023

Copy link
Copy Markdown
Member Author

Thanks @logicalhan! There’s a failure in apidiff but I don’t think it’s significant — the changes are expected (or bugs in apidiff).

@logicalhan

logicalhan commented Nov 23, 2023

Copy link
Copy Markdown

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 23, 2023
@logicalhan

logicalhan commented Nov 23, 2023

Copy link
Copy Markdown

Thanks @logicalhan! There’s a failure in apidiff but I don’t think it’s significant — the changes are expected (or bugs in apidiff).

It is significant because we don't know who's using this.

@skitt

skitt commented Nov 23, 2023

Copy link
Copy Markdown
Member Author

It is significant because we don't know who's using this.

The changes are listed as follows:

- KeySet: changed from func(map[E]A) Set[E] to func(map[E]A) Set[E]
- New: changed from func(...E) Set[E] to func(...E) Set[E]
- Set: changed from Set[E ordered] to Set[E comparable]

The first two aren’t changes. The last one is fine because comparable is a superset of ordered, so any existing Set will still build.

@wallrj wallrj mentioned this pull request Jan 2, 2024
Merged
2 tasks
@k8s-triage-robot

k8s-triage-robot commented Feb 21, 2024

Copy link
Copy Markdown

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 21, 2024
@skitt

skitt commented Feb 21, 2024

Copy link
Copy Markdown
Member Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 21, 2024
@k8s-triage-robot

k8s-triage-robot commented May 21, 2024

Copy link
Copy Markdown

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 21, 2024
@skitt

skitt commented May 22, 2024

Copy link
Copy Markdown
Member Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 22, 2024
@k8s-triage-robot

k8s-triage-robot commented Aug 20, 2024

Copy link
Copy Markdown

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 20, 2024
@skitt

skitt commented Aug 20, 2024

Copy link
Copy Markdown
Member Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 20, 2024
@skitt skitt mentioned this pull request Oct 7, 2024
Open
4 tasks
@k8s-triage-robot

k8s-triage-robot commented Nov 18, 2024

Copy link
Copy Markdown

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 18, 2024
@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 17, 2025
@skitt

skitt commented Jun 17, 2025

Copy link
Copy Markdown
Member Author

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jun 17, 2025
@k8s-triage-robot

k8s-triage-robot commented Sep 15, 2025

Copy link
Copy Markdown

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 15, 2025
@skitt

skitt commented Sep 15, 2025

Copy link
Copy Markdown
Member Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 15, 2025
@k8s-triage-robot

k8s-triage-robot commented Dec 14, 2025

Copy link
Copy Markdown

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 14, 2025
@skitt

skitt commented Dec 15, 2025

Copy link
Copy Markdown
Member Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 15, 2025
@skitt skitt force-pushed the align-apimachinery-set branch from 93a9af8 to 9ec58aa Compare December 18, 2025 17:05
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 18, 2025
@k8s-ci-robot

k8s-ci-robot commented Dec 18, 2025

Copy link
Copy Markdown
Contributor

New changes are detected. LGTM label has been removed.

@skitt
Align set with k/k apimachinery
9156a7c 
This aligns the features of generic Set with the version in
apimachinery. The main change is that sets can be instantiated with
any comparable type, not only ordered types; as a result, SortedList()
can no longer be implemented as a method, and is replaced by the
List() function to match apimachinery.

Signed-off-by: Stephen Kitt <skitt@redhat.com>
@skitt skitt force-pushed the align-apimachinery-set branch from 9ec58aa to 9156a7c Compare March 12, 2026 15:30
@k8s-ci-robot

k8s-ci-robot commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Details 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

@k8s-ci-robot k8s-ci-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 12, 2026
@dims

dims commented Mar 12, 2026

Copy link
Copy Markdown
Member

/assign @pohly @liggitt

@skitt

skitt commented Mar 12, 2026

Copy link
Copy Markdown
Member Author

The apidiff failure is

FAIL: k8s.io/utils/set contains incompatible changes:
- KeySet: changed from func(map[E]A) Set[E] to func(map[E]A) Set[E]
- New: changed from func(...E) Set[E] to func(...E) Set[E]
- Set: changed from Set[E ordered] to Set[E comparable]

The change it’s complaining about is the last one, which percolates up through the stack. But in practice I don’t think it’s a breaking API change: current code using k/u Set with an ordered type will compile fine with the new one, since an ordered type is also comparable.

@skitt
Drop ordered in favour of cmp.Ordered
9ae1dad 
Go 1.21+ provides cmp.Ordered which serves the same purpose as set's
private ordered type.

Signed-off-by: Stephen Kitt <skitt@redhat.com>
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 13, 2026
@k8s-triage-robot

k8s-triage-robot commented Jun 11, 2026

Copy link
Copy Markdown

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 11, 2026
@skitt

skitt commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thockin thockin Awaiting requested review from thockin

1 more reviewer

@logicalhan logicalhan logicalhan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@pohly pohly

@logicalhan logicalhan

@liggitt liggitt

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@skitt @logicalhan @k8s-triage-robot @k8s-ci-robot @dims @pohly @benjaminapetersen @liggitt