Encapsulating State access by moving nodes to runtime.state package #12314
+372
−132
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JaroslavTulach
requested review from
4e6,
hubertp and
Akirathan
as code owners
JaroslavTulach
added
the
CI: No changelog needed
JaroslavTulach
requested review from
jdunkerley,
radeusgd,
GregoryTravis,
AdRiley and
marthasharkey
as code owners
JaroslavTulach
commented
Feb 19, 2025
| main = | ||
| x_0 = 6 | ||
| x_1 = 7 | ||
| x_2 = (v "${aa}")*(v "${bb}") |
jdunkerley
approved these changes
Feb 19, 2025
Comment on lines
+56
to
+57
| value_for_uuid id = instrumentor_builtin "uuid" id | ||
| instrumentor_builtin op args = @Builtin_Method "Meta.instrumentor_builtin" |
There was a problem hiding this comment.
Do we not need these in the full distribution too?
There was a problem hiding this comment.
The whole concept of micro distribution is flawed
- it is just a random selection of stuff from
Standard.Baseto make sure some unit tests work - ideally we have
Standard.Preludeneeded asmain = 6 * 7no longer works #8852 - the prelude would be a subset of
Standard.Base - the micro distribution is deleted and prelude is used instead
Until that happens, we just have to update micro-distribution to be on par with builtins used by Standard.Base.
JaroslavTulach
force-pushed
the
wip/jtulach/StateNodes12294
branch
from
5c4a006 to
d379995
Compare
GregoryTravis
approved these changes
Feb 19, 2025
4e6
reviewed
Feb 19, 2025
| new Runtime$Api$MethodPointer(moduleName, moduleName, "main"), | ||
| Option.empty(), | ||
| ScalaConversions.<String>nil().toVector())))); | ||
| var reply = context.receiveN(5, 3); |
There was a problem hiding this comment.
Please don't override the timeout. Otherwise, it may randomly fail on CI.
There was a problem hiding this comment.
I am not overriding timeout, but Java doesn't support default Scala arguments. I have to specify one.
| * new Java instead of in Scala. | ||
| */ | ||
| final class RuntimeServerTesting { | ||
| static void accessRuntimeCache(TestContext context) { |
There was a problem hiding this comment.
I'd just make a new Junit test suite
There was a problem hiding this comment.
That wouldn't inherit the infrastructure of RuntimeServerTest setup and teardown.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
Refactoring to fix #12294.
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,