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

ref(client): Debug, Eq, Hash, PartialEq for ImageLayer; convert annotations to BTreeMap #121

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

vdice
Copy link
Contributor

@vdice vdice commented Mar 29, 2024

Changeset motivated by the need to differentiate and deduplicate collections of ImageLayers.

Copy link
Contributor

@thomastaylor312 thomastaylor312 left a comment

Choose a reason for hiding this comment

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

Great change here! One comment around changing the type and just breaking the API

src/client.rs Outdated Show resolved Hide resolved
@vdice vdice force-pushed the feat/image-layer-traits branch 2 times, most recently from ef3d433 to 3eb6340 Compare March 29, 2024 20:54
@vdice vdice changed the title feat(client): ImageLayer: derive Debug, Eq, PartialEq; impl Hash trait feat(client): ImageLayer: derive Debug, Eq, Hash, PartialEq Mar 29, 2024
@vdice vdice force-pushed the feat/image-layer-traits branch from 3eb6340 to 48f5ebc Compare March 29, 2024 20:56
Copy link
Contributor

@thomastaylor312 thomastaylor312 left a comment

Choose a reason for hiding this comment

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

Thanks for the great work here! @flavio would like your review too before I merge since this is a breaking API change.

@thomastaylor312
Copy link
Contributor

@vdice heads up that it looks like the BTreeMap import didn't get picked up in the windows build for some reason

@vdice vdice force-pushed the feat/image-layer-traits branch from 48f5ebc to 30a608e Compare March 29, 2024 21:27
@vdice
Copy link
Contributor Author

vdice commented Mar 29, 2024

@thomastaylor312 thanks - looks like I didn't update the import in examples/wasm/push.rs. Now updated; hopefully that was the issue.

edit: nope; still failing... looking

edit 2: okay, needed to update examples/wasm/main.rs as well. Weird - nothing complained when I ran my usual just build && just test flow... but once I started running the cargo test command from the justfile-windows file, I was able to repro. Turns out the warnings also occur on main, but at least there should be no error now.

- eliminates the need for a specific Hash impl for ImageLayer

Signed-off-by: Vaughn Dice <[email protected]>
@vdice vdice force-pushed the feat/image-layer-traits branch from 30a608e to dfe6d40 Compare March 29, 2024 21:41
@vdice vdice changed the title feat(client): ImageLayer: derive Debug, Eq, Hash, PartialEq ref(client): Debug, Eq, Hash, PartialEq for ImageLayer; convert annotations to BTreeMap Mar 29, 2024
@thomastaylor312
Copy link
Contributor

Thanks for soldering through that @vdice. Just waiting on a second opinion from @flavio on the breaking aspect of this

@flavio
Copy link
Contributor

flavio commented Apr 2, 2024

Sorry, I was on vacation. This change looks good to me

@flavio flavio merged commit f70874f into oras-project:main Apr 2, 2024
5 checks passed
@vdice vdice deleted the feat/image-layer-traits branch April 2, 2024 14:58
@vdice
Copy link
Contributor Author

vdice commented Apr 2, 2024

Thanks so much @thomastaylor312 and @flavio!

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

Successfully merging this pull request may close these issues.

3 participants