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

Rename toString to toStringSigned #4330

Conversation

balajipachai
Copy link
Contributor

Fixes #4298

  • Adds a StringsMock contract
  • From within the StringsMock contract's constructor a call is made to Strings.toStringSigned(100)
  • Added tests and it is passing

PR Checklist

  • [All Passing] Tests
  • [No Change] Documentation
  • [None] Changeset entry (run npx changeset add)

* adds StringsMock contract
* updates Strings contract & tests
@changeset-bot
Copy link

changeset-bot bot commented Jun 9, 2023

🦋 Changeset detected

Latest commit: 2a39ea1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
openzeppelin-solidity Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Amxx
Copy link
Collaborator

Amxx commented Jun 9, 2023

We don't need a StringsMock, we use hardat-exposed to generate the test contracts.

Copy link
Collaborator

@Amxx Amxx left a comment

Choose a reason for hiding this comment

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

This is not fixing the PR linked.

@balajipachai
Copy link
Contributor Author

balajipachai commented Jun 9, 2023 via email

@frangio
Copy link
Contributor

frangio commented Jun 15, 2023

This PR is not right. As #4298 says, we have to rename toString(int256) into toStringSigned(int256).

@balajipachai
Copy link
Contributor Author

This PR is not right. As #4298 says, we have to rename toString(int256) into toStringSigned(int256).

Ok, ok, got it, I renamed uint256, I will update, thank you.

balajipachai and others added 3 commits June 16, 2023 14:58
Co-authored-by: Francisco <fg@frang.io>
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Copy link
Collaborator

@Amxx Amxx left a comment

Choose a reason for hiding this comment

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

Code is good, but we need a changeset entry.

@balajipachai
Copy link
Contributor Author

Code is good, but we need a changeset entry.

Will add one

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

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

Thanks!

This was referenced Sep 10, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strings.toString doesn't compile with integer literals
3 participants