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

Extend parse send args tests #899

Merged
merged 23 commits into from
Apr 8, 2024
Merged

Extend parse send args tests #899

merged 23 commits into from
Apr 8, 2024

Conversation

zancas
Copy link
Member

@zancas zancas commented Apr 6, 2024

Here are more tests for the parse_send_args helper utility.

There are a few improvements:

  • more error variants
  • trim against from-single-arg int parsing

@fluidvanadium
Copy link
Contributor

noticed, but deferring to oscar

Oscar-Pepper
Oscar-Pepper previously requested changes Apr 8, 2024
View reviewed changes
Copy link
Contributor

@Oscar-Pepper Oscar-Pepper left a comment

Choose a reason for hiding this comment

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

Good changes and really like the tests of each error variant. I have some change requests / questions

zingolib/src/commands/utils.rs Outdated Show resolved Hide resolved
zingolib/src/commands/utils.rs Outdated Show resolved Hide resolved
zingolib/src/commands/utils.rs Outdated Show resolved Hide resolved
zingolib/src/commands/utils.rs Show resolved Hide resolved
zingolib/src/commands/utils.rs Outdated Show resolved Hide resolved
zingolib/src/commands/utils.rs Outdated Show resolved Hide resolved
zingolib/src/commands/utils.rs Outdated Show resolved Hide resolved
zingolib/src/commands/utils.rs Outdated Show resolved Hide resolved
zingolib/src/commands/error.rs Outdated Show resolved Hide resolved
zingolib/src/commands/error.rs Outdated Show resolved Hide resolved
zancas and others added 12 commits April 8, 2024 12:28
@zancas @Oscar-Pepper
Update zingolib/src/commands/utils.rs
3d12b33 
Co-authored-by: oscar-pepper <109323234+Oscar-Pepper@users.noreply.github.com>
Signed-off-by: Za Wilcox <zancas@zingolabs.org>
@zancas @Oscar-Pepper
Update zingolib/src/commands/error.rs
6130f87 
Co-authored-by: oscar-pepper <109323234+Oscar-Pepper@users.noreply.github.com>
Signed-off-by: Za Wilcox <zancas@zingolabs.org>
@zancas
removed redundant forbid_unsafe
c07505d
@zancas
move error description into fmt implementation, and clarify
b7224d9
@zancas
prefer explicit imports and match unit test name to fn lrz-style
64e0d12
@zancas
rm dbg
f0f7854
@zancas
rm dgbs
0f196d3
@zancas
match lrz test name convention
87b9b35
@zancas
move logic for successful behavior check into parse_send_args lrz-style
af651a9
@zancas
fix unused, rm redundant test
793d6c1
@zancas @Oscar-Pepper
Update zingolib/src/commands/error.rs
8b4fc8b 
Co-authored-by: oscar-pepper <109323234+Oscar-Pepper@users.noreply.github.com>
Signed-off-by: Za Wilcox <zancas@zingolabs.org>
@zancas
apply fmt fixup
13d3587
@zancas zancas requested a review from Oscar-Pepper April 8, 2024 19:23
@zancas zancas dismissed Oscar-Pepper’s stale review April 8, 2024 19:46

I think I have addressed all requested issues.

Oscar-Pepper
Oscar-Pepper approved these changes Apr 8, 2024
View reviewed changes
Copy link
Contributor

@Oscar-Pepper Oscar-Pepper left a comment

Choose a reason for hiding this comment

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

splendid

@Oscar-Pepper Oscar-Pepper merged commit e3d36aa into zingolabs:dev Apr 8, 2024
17 checks passed
@zancas zancas deleted the extend_parse_send_args_tests branch May 30, 2024 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Oscar-Pepper Oscar-Pepper Oscar-Pepper approved these changes

@fluidvanadium fluidvanadium Awaiting requested review from fluidvanadium

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

Successfully merging this pull request may close these issues.
3 participants
@zancas @fluidvanadium @Oscar-Pepper