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.

Sign up for GitHub

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

Jump to bottom

feat: Provide native way to cast deps to empty #1813

Merged
merged 1 commit into from
Aug 15, 2023
Merged

feat: Provide native way to cast deps to empty #1813

merged 1 commit into from
Aug 15, 2023

Conversation

jawoznia
Copy link
Contributor

closes #1812

jawoznia
jawoznia commented Aug 14, 2023
View reviewed changes
packages/std/src/deps.rs Outdated Show resolved Hide resolved
webmaster128
webmaster128 reviewed Aug 14, 2023
View reviewed changes
Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Nice start, thanks. Some thoughts here:

  • to -> into
  • is into_empty a good name? Ideas @chipshort?
  • We need doc comments for the new functions to help contract developers understand what this does and if they should use it.

packages/std/src/traits.rs Outdated Show resolved Hide resolved
chipshort
chipshort reviewed Aug 15, 2023
View reviewed changes
packages/std/src/deps.rs Outdated Show resolved Hide resolved
packages/std/src/deps.rs Outdated Show resolved Hide resolved
@jawoznia jawoznia force-pushed the cast_deps_to_empty branch 2 times, most recently from 3f96a92 to 17f7011 Compare August 15, 2023 10:53
chipshort
chipshort approved these changes Aug 15, 2023
View reviewed changes
Copy link
Collaborator

@chipshort chipshort left a comment

Choose a reason for hiding this comment

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

LGTM

webmaster128
webmaster128 reviewed Aug 15, 2023
View reviewed changes
Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Diff LGTM. Could you add a brief CHANGELOG entry to the Unreleased section in CHANGELOG.md?

@jawoznia jawoznia force-pushed the cast_deps_to_empty branch 3 times, most recently from c298e1c to 5741162 Compare August 15, 2023 13:10
@jawoznia jawoznia merged commit cb4a6a6 into main Aug 15, 2023
3 checks passed
@jawoznia jawoznia deleted the cast_deps_to_empty branch August 15, 2023 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@webmaster128 webmaster128 webmaster128 left review comments

@chipshort chipshort chipshort approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide native way to cast Deps(Mut)<Custom> to Deps(Mut)<Empty>
3 participants
@jawoznia @webmaster128 @chipshort