Skip to content

feat(dynamic): add dynamic wrapper function#5242

Merged
onelson merged 7 commits intomasterfrom
onelson/feat/dynamic-wrapper
Sep 29, 2022
Merged

feat(dynamic): add dynamic wrapper function#5242
onelson merged 7 commits intomasterfrom
onelson/feat/dynamic-wrapper

Conversation

@onelson
Copy link
Contributor

@onelson onelson commented Sep 29, 2022

Originally planned as a part of #5143, this diff adds a new builtin to the experimental/dynamic package which wraps an existing value in a Dynamic.

This implementation is weak, probably incomplete, and hard to test without additional work since part of the contract of our "restrictive" dynamic type is you can't really do much with it until you cast it to some other value.
The buried lede is: we haven't implemented the dynamic support for the standard cast fns yet.

At this early stage the best I can offer in terms of testing is this short REPL session:

❯ go run ./cmd/flux
> import dyn "experimental/dynamic"
> dyn.dynamic(v: 123)
<unknown value>
> dyn.dynamic(v: 123) == dyn.dynamic(v: 123)
Error: unsupported binary expression dynamic == dynamic
>

I went ahead and added the == case to the test file just so we can get the addition of the file in the repo, and to have some sort of test in place. There's not a whole lot we can do right now other than prove the function is something you can import.

Checklist

Dear Author 👋, the following checks should be completed (or explicitly dismissed) before merging.

  • ✏️ Write a PR description, regardless of triviality, to include the value of this PR
  • 🔗 Reference related issues
  • 🏃 Test cases are included to exercise the new code
  • 🧪 If new packages are being introduced to stdlib, link to Working Group discussion notes and ensure it lands under experimental/
  • 📖 If language features are changing, ensure docs/Spec.md has been updated

Dear Reviewer(s) 👋, you are responsible (among others) for ensuring the completeness and quality of the above before approval.

@onelson onelson force-pushed the onelson/feat/dynamic-wrapper branch from 6b0877e to 90d17b9 Compare September 29, 2022 16:16
@onelson onelson force-pushed the onelson/feat/dynamic-wrapper branch from 90d17b9 to 7b3a5c0 Compare September 29, 2022 16:46
Owen Nelson added 2 commits September 29, 2022 10:07
Not really a good test. At this point, just verifying it can be imported
and recieve an argument :(
@onelson onelson force-pushed the onelson/feat/dynamic-wrapper branch from 7b3a5c0 to 6814e80 Compare September 29, 2022 17:08
@onelson onelson requested a review from wolffcm September 29, 2022 17:14
@onelson onelson marked this pull request as ready for review September 29, 2022 17:14
@onelson onelson requested review from a team as code owners September 29, 2022 17:14
@onelson onelson requested review from lwandzura and removed request for a team September 29, 2022 17:14
@sanderson sanderson requested review from sanderson and removed request for lwandzura September 29, 2022 17:14
Copy link

@wolffcm wolffcm left a comment

Choose a reason for hiding this comment

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

Looks great.

@onelson
Copy link
Contributor Author

onelson commented Sep 29, 2022

Apologies to @sanderson. I'd like to offer some practical examples in the docstring, but nothing will actually fly atm. I'll aim to loop back as we flesh out the functionality.

Copy link
Contributor

@sanderson sanderson left a comment

Choose a reason for hiding this comment

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

Just some minor suggestions. Looks really good.

@onelson onelson force-pushed the onelson/feat/dynamic-wrapper branch from 52d82ca to a15db9a Compare September 29, 2022 17:23
Owen Nelson and others added 4 commits September 29, 2022 10:31
@onelson onelson force-pushed the onelson/feat/dynamic-wrapper branch from a15db9a to 3e88f65 Compare September 29, 2022 17:31
@onelson onelson requested a review from sanderson September 29, 2022 17:47
Copy link
Contributor

@sanderson sanderson left a comment

Choose a reason for hiding this comment

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

👌🏻 Looks great.

@onelson onelson merged commit 940f882 into master Sep 29, 2022
@onelson onelson deleted the onelson/feat/dynamic-wrapper branch September 29, 2022 19:08
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.

3 participants