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.

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

Implement 'dec' for converting integers to decimals #1150

Merged
merged 13 commits into from
May 1, 2023
Merged

Conversation

jwiegley
Copy link
Contributor

No description provided.

emilypi
emilypi previously approved these changes Feb 24, 2023
Copy link
Member

@emilypi emilypi left a comment

Choose a reason for hiding this comment

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

LGTM! Nice.

@jwiegley
Copy link
Contributor Author

I'm not happy with the naming of types in this PR. Expect changes soon.

@jwiegley
Copy link
Contributor Author

Also, this isn't something Pact developers should start using, if they can't deploy it on chain for another three months. A better answer for now may be to disable the warning -- or allow it to be disabled -- until this mitigation can appear on chain.

Copy link
Member

@emilypi emilypi left a comment

Choose a reason for hiding this comment

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

Beautiful. Can we get some regressions added to ops.repl just to make sure the Pact outputs are covered?

@jwiegley jwiegley requested a review from jmcardon as a code owner May 1, 2023 18:38
@jwiegley
Copy link
Contributor Author

jwiegley commented May 1, 2023

Thank you for reminding me @emilypi, @rsoeldner is currently working on some tests that we will merge into this PR.

* add tests

* fix int to dec casting within fv
@jwiegley
Copy link
Contributor Author

jwiegley commented May 1, 2023

This native needs to be gated on FlagDisablePact47 per Jose's versioned natives work.

@jmcardon
Copy link
Member

jmcardon commented May 1, 2023

I will add the forking in #1195

@jmcardon jmcardon merged commit 81eadfd into master May 1, 2023
6 checks passed
@emilypi emilypi mentioned this pull request May 4, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants