Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Add tests for implementing the Eq trait for Address #672

Merged
merged 12 commits into from
Jan 28, 2022
Merged

Add tests for implementing the Eq trait for Address #672

merged 12 commits into from
Jan 28, 2022

Conversation

nfurfaro
Copy link
Contributor

This adds a test for new code PRd here: FuelLabs/sway-lib-std#25
CI will fail until the above PR is merged.

@nfurfaro nfurfaro added the lib: std Standard library label Jan 19, 2022
@nfurfaro nfurfaro requested a review from sezna as a code owner January 19, 2022 22:21
@nfurfaro nfurfaro self-assigned this Jan 19, 2022
@nfurfaro nfurfaro mentioned this pull request Jan 19, 2022
Merged
@nfurfaro
Copy link
Contributor Author

Blocked by FuelLabs/sway-lib-core#8

@nfurfaro nfurfaro marked this pull request as draft January 20, 2022 15:37
adlerjohn
adlerjohn reviewed Jan 20, 2022
View reviewed changes
Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

Could you also add corner-cases tests? E.g. if the upper words are the same, but the lower 3 words have a difference, then it should not be equal.

@nfurfaro nfurfaro changed the title Add test for Ord::eq for Address type Add tests for implementing the Eq trait for Address Jan 25, 2022
@nfurfaro nfurfaro marked this pull request as ready for review January 25, 2022 18:38
@nfurfaro nfurfaro requested a review from adlerjohn January 26, 2022 00:17
@nfurfaro nfurfaro requested a review from adlerjohn January 27, 2022 17:11
adlerjohn
adlerjohn approved these changes Jan 27, 2022
View reviewed changes
Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

LGTM, but will defer to @sezna

@sezna sezna merged commit ff94ef0 into master Jan 28, 2022
@sezna sezna deleted the furnic/impl-ord-address branch January 28, 2022 16:32
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@adlerjohn adlerjohn adlerjohn approved these changes

@sezna sezna Awaiting requested review from sezna

Assignees

@nfurfaro nfurfaro

Labels
lib: std Standard library
Projects
Fuel Network
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nfurfaro @adlerjohn @sezna