Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

refactor!: Make KeyValuePairs an alias for BTreeMap#889

Open
nightkr wants to merge 15 commits into
main from
feature/kvp-reorg
Open

refactor!: Make KeyValuePairs an alias for BTreeMap #889
nightkr wants to merge 15 commits into
main from
feature/kvp-reorg

Conversation

@nightkr

@nightkr nightkr commented Oct 14, 2024
edited by Techassi
Loading

Copy link
Copy Markdown
Contributor

This is effectively an extended version of #888 (and supersedes it).

@nightkr nightkr linked an issue Oct 14, 2024 that may be closed by this pull request

@Techassi Techassi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Very nice job, I like it!

Left a bunch of small comments, nothing major.

Comment thread crates/stackable-operator/src/kvp/annotation/mod.rs
Comment thread crates/stackable-operator/src/kvp/key.rs Outdated
Comment thread crates/stackable-operator/src/kvp/label/mod.rs
Comment thread crates/stackable-operator/src/kvp/mod.rs
Comment on lines +24 to +27
#[cfg(doc)]
use k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta;
#[cfg(doc)]
use std::ops::Deref;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

suggestion: Move these imports to the std library and external crate imports at the top of the file.

@nightkr nightkr Oct 16, 2024
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it makes sense to call out doc-only "fake" imports as different from regular one (just like publics), though we definitely haven't done a great job at consistency around that in general.

Comment thread crates/stackable-operator/src/kvp/mod.rs Outdated
Comment thread crates/stackable-operator/src/kvp/mod.rs Outdated
Comment thread crates/stackable-operator/src/kvp/mod.rs Outdated
Comment thread crates/stackable-operator/CHANGELOG.md
Comment thread crates/stackable-operator/CHANGELOG.md Outdated
@Techassi Techassi changed the title (削除) Reorganize kvp so that KeyValuePairs is an alias for BTreeMap instead of newtyping it (削除ここまで) (追記) refactor!: Make KeyValuePairs an alias for BTreeMap (追記ここまで) Aug 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Techassi Techassi Techassi requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

KeyValuePairs::extend implements a nonsense merge

2 participants

AltStyle によって変換されたページ (->オリジナル) /