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: Modify parseUint function to use strconv package #358

Merged
merged 7 commits into from
Nov 18, 2024
Merged

feat: Modify parseUint function to use strconv package #358

merged 7 commits into from
Nov 18, 2024

Conversation

notJoon
Copy link
Member

@notJoon notJoon commented Oct 21, 2024

Description

Apply strconv package

@notJoon notJoon marked this pull request as ready for review October 21, 2024 06:12
@r3v4s
Copy link
Member

r3v4s commented Oct 21, 2024 •
edited
Loading

Code looks good, but to merge I think we should wait for
gnolang/gno#1464 <- this change to be applied to gno's test4

@notJoon
Copy link
Member Author

notJoon commented Oct 21, 2024

set as draft until gno binary contains strconv support version

@notJoon notJoon marked this pull request as draft October 21, 2024 06:27
@dongwon8247
Copy link
Member

@notJoon Let's merge this since the next gno binary for test5 will include this. Our devnet reset tomorrow should include this.

@notJoon notJoon marked this pull request as ready for review November 7, 2024 09:08
@notJoon
Copy link
Member Author

notJoon commented Nov 7, 2024

@notJoon Let's merge this since the next gno binary for test5 will include this. Our devnet reset tomorrow should include this.

Good. I'll merge this after resolving some conflicts 👍

dongwon8247
dongwon8247 previously approved these changes Nov 7, 2024
View reviewed changes
r3v4s
r3v4s requested changes Nov 7, 2024
View reviewed changes
Copy link
Member

@r3v4s r3v4s left a comment

Choose a reason for hiding this comment

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

@notJoon tc fails, please make sure all of parseUint to use from strconv.

@notJoon notJoon requested a review from r3v4s November 7, 2024 09:26
@notJoon
Copy link
Member Author

notJoon commented Nov 7, 2024

@r3v4s wrong click. I'll ping you after fixing them

@notJoon
Copy link
Member Author

notJoon commented Nov 7, 2024

@r3v4s It has been corrected. The files were mixed up while merging process.

r3v4s
r3v4s reviewed Nov 7, 2024
View reviewed changes
gov/governance/execute.gno Outdated Show resolved Hide resolved
r3v4s
r3v4s reviewed Nov 7, 2024
View reviewed changes
gov/governance/execute.gno Outdated Show resolved Hide resolved
r3v4s
r3v4s reviewed Nov 7, 2024
View reviewed changes
Copy link
Member

@r3v4s r3v4s left a comment

Choose a reason for hiding this comment

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

2 small comments, please address them.

@notJoon notJoon requested a review from r3v4s November 18, 2024 01:42
@notJoon notJoon merged commit 6593753 into main Nov 18, 2024
@notJoon notJoon deleted the strconv branch November 18, 2024 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@r3v4s r3v4s r3v4s approved these changes

@dongwon8247 dongwon8247 dongwon8247 approved these changes

@onlyhyde onlyhyde Awaiting requested review from onlyhyde

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
@notJoon @r3v4s @dongwon8247