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

Remove slice() length documentation caveats #3152

Merged
merged 1 commit into from Nov 17, 2022
Merged

Remove slice() length documentation caveats #3152

merged 1 commit into from Nov 17, 2022

Conversation

ghost
Copy link

@ghost ghost commented Nov 16, 2022

What I did

Removed the caveats on not being allowed to use variables/immutables as length arguments for slice()

I think these applied in a previous version of Vyper, but as of at least v0.3.7 the restriction doesn't seem to apply anymore

How to verify it

Tests with variables for length arguments exist already in tests/parser/functions/test_slice.py, such as test_basic_slice()

A test_slice_immutable_length_arg() was also added to demonstrate that the functionality is available

Commit message

Remove slice() length documentation caveats

Description for the changelog

Remove slice() length documentation caveats

Cute Animal Picture

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

@codecov-commenter
Copy link

Codecov Report

Merging #3152 (96b588f) into master (fb50017) will decrease coverage by 37.36%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #3152       +/-   ##
===========================================
- Coverage   88.49%   51.13%   -37.37%     
===========================================
  Files          95       95               
  Lines       10778    10774        -4     
  Branches     2268     2202       -66     
===========================================
- Hits         9538     5509     -4029     
- Misses        797     4734     +3937     
- Partials      443      531       +88     
Impacted Files Coverage Δ
vyper/builtin_functions/utils.py 0.00% <0.00%> (-100.00%) ⬇️
vyper/builtin_interfaces/ERC165.py 0.00% <0.00%> (-100.00%) ⬇️
vyper/builtin_interfaces/ERC4626.py 0.00% <0.00%> (-100.00%) ⬇️
vyper/builtin_interfaces/ERC20Detailed.py 0.00% <0.00%> (-100.00%) ⬇️
vyper/ir/s_expressions.py 5.88% <0.00%> (-88.24%) ⬇️
vyper/ast/natspec.py 12.34% <0.00%> (-86.42%) ⬇️
vyper/codegen/self_call.py 17.39% <0.00%> (-78.27%) ⬇️
vyper/codegen/external_call.py 19.29% <0.00%> (-78.08%) ⬇️
vyper/semantics/validation/levenshtein_utils.py 16.00% <0.00%> (-76.00%) ⬇️
vyper/ast/expansion.py 20.00% <0.00%> (-75.00%) ⬇️
... and 62 more

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

@ghost
Copy link
Author

ghost commented Nov 16, 2022

Merging #3152 (96b588f) into master (fb50017) will decrease coverage by 37.36%.

Uhhh not sure about that 👀 issue with the bot maybe?

@fubuloubu
Copy link
Member

Merging #3152 (96b588f) into master (fb50017) will decrease coverage by 37.36%.

Uhhh not sure about that 👀 issue with the bot maybe?

Some of the test suite is split up across multiple CI, it updates whenever the first one passes, and then keeps updating as other passes report status

@fubuloubu fubuloubu merged commit 80dcf5e into vyperlang:master Nov 17, 2022
@ghost ghost deleted the fix/slice-docs branch November 17, 2022 16:12
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.

2 participants