vyper
vyper copied to clipboard
vyperlang
feat: change ABI type of `decimal` to `int168`
going forward, vyper should expose decimal types as integers in the ABI.
What I did
change the type of decimal in the ABI to int168. in the future, the syntax for decimal will also change (to something like Decimal[<bits>, <places>])
How I did it
How to verify it
Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Cute Animal Picture
note there will probably be some tests that need to be rewritten to use wad-type things instead of floats/decimals
Please adjust the docs as well: https://github.com/vyperlang/vyper/blob/master/docs/types.rst?plain=1#L240. Also, I'm wondering why no tests have to be adjusted here, i.e. there is a missing test coverage here, that ensures the correct ABI type for
decimal. We should add such a test as well IMO.
i don't think there are missing tests -- you can see there are failing tests which i haven't updated yet
Please adjust the docs as well: https://github.com/vyperlang/vyper/blob/master/docs/types.rst?plain=1#L240. Also, I'm wondering why no tests have to be adjusted here, i.e. there is a missing test coverage here, that ensures the correct ABI type for
decimal. We should add such a test as well IMO.i don't think there are missing tests -- you can see there are failing tests which i haven't updated yet
Oh yeah, weird, my GH UI didn't display the CI checks previously...
also re. the docs -- i've been thinking maybe we should move them to a separate repo?
also re. the docs -- i've been thinking maybe we should move them to a separate repo?
I'm not fully convinced this is a good idea. Vyper docs are still lacking a lot of insights, and having them part of the main repo helps to bundle the code and documentation changes in on PR. I'm afraid, that in the main repo a lot of undocumented activity will happen since maintaining a separate docs repo creates overhead.
I'm not fully convinced this is a good idea. Vyper docs are still lacking a lot of insights, and having them part of the main repo helps to bundle the code and documentation changes in on PR. I'm afraid, that in the main repo a lot of undocumented activity will happen since maintaining a separate docs repo creates overhead.
i think it could actually decrease overhead, because docs changes would not need to go through the review and CI process (the latter of which can take 30-60 minutes depending on CI busy-ness)
i think it could actually decrease overhead, because docs changes would not need to go through the review and CI process (the latter of which can take 30-60 minutes depending on CI busy-ness)
I think this is a non-issue, and having a review of the docs means additional QA in the context of the code-changing PR. If it's just a small documentation fix (w/o code change), 60 mins of CI don't matter, really. I personally think the current as-is setup is more streamlined, particularly given the team size.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 90.78%. Comparing base (
eb81c26) to head (f9c9f4a).
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## master #3696 +/- ##
==========================================
- Coverage 90.93% 90.78% -0.15%
==========================================
Files 95 95
Lines 14411 14397 -14
Branches 3193 3191 -2
==========================================
- Hits 13105 13071 -34
- Misses 906 924 +18
- Partials 400 402 +2
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
