Skip to content

fix: do not mutate std.removeAt parameters#812

Merged
johnbartholomew merged 1 commit into
google:masterfrom
zarelit:fix_807_removeAt
Jan 27, 2026
Merged

fix: do not mutate std.removeAt parameters#812
johnbartholomew merged 1 commit into
google:masterfrom
zarelit:fix_807_removeAt

Conversation

@zarelit
Copy link
Copy Markdown
Contributor

@zarelit zarelit commented Jun 18, 2025

Resolves #807

@zarelit zarelit force-pushed the fix_807_removeAt branch from 794cad0 to b961f92 Compare June 19, 2025 07:17
stephenamar-db pushed a commit to databricks/sjsonnet that referenced this pull request Jun 19, 2025
…r. (#437)

Motivation:
Add a test for std.removeAt, which should not modify the parameter.

ported from go-jsonnet 
refs: google/go-jsonnet#812
@zarelit
Copy link
Copy Markdown
Contributor Author

zarelit commented Jan 27, 2026

@johnbartholomew may I kindly ask for your review?

Copy link
Copy Markdown
Collaborator

@johnbartholomew johnbartholomew left a comment

Choose a reason for hiding this comment

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

Looks good, just a file formatting nit (missing newline at end of file).

Comment thread builtins.go
return nil, err
}

newArr := append(arr.elements[:idx], arr.elements[idx+1:]...)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Go's funky append() behaviour strikes again.

Comment thread testdata/builtinRemoveAt2.jsonnet Outdated
@coveralls
Copy link
Copy Markdown

coveralls commented Jan 27, 2026

Coverage Status

coverage: 44.277% (+0.008%) from 44.269%
when pulling 268c15a on zarelit:fix_807_removeAt
into 23a013c on google:master.

@johnbartholomew johnbartholomew merged commit c01b909 into google:master Jan 27, 2026
9 checks passed
@johnbartholomew
Copy link
Copy Markdown
Collaborator

Thanks for the fix!

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.

removeAt performs mutation on its parameter

3 participants