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

feat: remove builtin constants #3350

Merged
merged 6 commits into from
Oct 17, 2023

Conversation

tserg
Copy link
Collaborator

@tserg tserg commented Apr 10, 2023

What I did

Builtin constants like MAX_INT128, MIN_INT128, MIN_DECIMAL, MAX_DECIMAL and MAX_UINT256 have been deprecated since v0.3.4 with the introduction of the min_value and max_value builtins, and further back for ZERO_ADDRESS and EMPTY_BYTES32 with the empty builtin.

This PR removes them from the language entirely, and will be a breaking change.

How I did it

Delete code.

How to verify it

See tests.

Commit message

feat: remove builtin constants

the current set of builtin constants are no longer needed since the 
introduction of `min_value`, `max_value` and `empty` builtins.

Description for the changelog

Remove builtin constants.

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@codecov-commenter
Copy link

codecov-commenter commented Apr 10, 2023

Codecov Report

Merging #3350 (caffd02) into master (68da04b) will increase coverage by 0.34%.
The diff coverage is n/a.

❗ Current head caffd02 differs from pull request most recent head 2483099. Consider uploading reports for the commit 2483099 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master    #3350      +/-   ##
==========================================
+ Coverage   88.66%   89.01%   +0.34%     
==========================================
  Files          86       86              
  Lines       11467    11440      -27     
  Branches     2607     2605       -2     
==========================================
+ Hits        10167    10183      +16     
+ Misses        873      835      -38     
+ Partials      427      422       -5     
Files Coverage Δ
vyper/ast/folding.py 92.72% <ø> (-0.56%) ⬇️
vyper/compiler/phases.py 93.02% <ø> (+0.65%) ⬆️

... and 17 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@charles-cooper
Copy link
Member

i'm pretty ok with merging this even for the 0.3.x series tbh. the old constants have been emitting warnings for nearly a year. @fubuloubu thoughts?

@fubuloubu
Copy link
Member

Sure, I think that seems realtively low-impact, but definitely worth broadcasting the breaking change

@charles-cooper charles-cooper added this to the v0.4.0 milestone Sep 25, 2023
@charles-cooper
Copy link
Member

nice cleanup!

@charles-cooper charles-cooper merged commit 5482bbc into vyperlang:master Oct 17, 2023
82 of 83 checks passed
@tserg tserg deleted the feat/remove_builtin_constants branch October 18, 2023 03:02
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