-
Notifications
You must be signed in to change notification settings - Fork 19
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
AERIE-2041 - TS convert functional programing UNIT to {} #311
Merged
goetzrrGit
merged 2 commits into
develop
from
AERIE-2041--TS-library-Computed-attributes-UNIT-TO-EMPTY
Sep 7, 2022
Merged
AERIE-2041 - TS convert functional programing UNIT to {} #311
goetzrrGit
merged 2 commits into
develop
from
AERIE-2041--TS-library-Computed-attributes-UNIT-TO-EMPTY
Sep 7, 2022
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
Twisol
reviewed
Sep 1, 2022
command-expansion-server/src/lib/codegen/ActivityTypescriptCodegen.ts
Outdated
Show resolved
Hide resolved
goetzrrGit
force-pushed
the
AERIE-2041--TS-library-Computed-attributes-UNIT-TO-EMPTY
branch
from
September 6, 2022 18:04
32f6dd3
to
eb5b35e
Compare
goetzrrGit
force-pushed
the
AERIE-2041--TS-library-Computed-attributes-UNIT-TO-EMPTY
branch
from
September 6, 2022 18:06
eb5b35e
to
3d164c8
Compare
goetzrrGit
force-pushed
the
AERIE-2041--TS-library-Computed-attributes-UNIT-TO-EMPTY
branch
from
September 6, 2022 19:12
3d164c8
to
f8998f7
Compare
Twisol
reviewed
Sep 6, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I might be jumping the gun a bit on this review; sorry if so.)
contrib/src/main/java/gov/nasa/jpl/aerie/contrib/serialization/rulesets/BasicValueMappers.java
Show resolved
Hide resolved
...essor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/generator/MissionModelGenerator.java
Outdated
Show resolved
Hide resolved
merlin-server/src/test/java/gov/nasa/jpl/aerie/merlin/server/models/MissionModelTest.java
Show resolved
Hide resolved
I learned a lot from this ticket. Now I found the code-gen part of Merlin. This gets propagated down to the command expansion part now and will show up as:
|
goetzrrGit
force-pushed
the
AERIE-2041--TS-library-Computed-attributes-UNIT-TO-EMPTY
branch
from
September 7, 2022 01:07
f8998f7
to
5addc85
Compare
Twisol
approved these changes
Sep 7, 2022
patkenneally
approved these changes
Sep 7, 2022
Add a Unit Value Mapper to map the UNIT value to an empty JAVA Map. This will be interpreted by the TS library generator as {} -- Summary -- In the mission model, the activity's effect model that doesn't return a computed attribute (aka void run() method of the activity) uses UNIT which represents a non-null empty value. This is used internally by AERIE https://en.wikipedia.org/wiki/Unit_type This UNIT value will be exposed to our users when command authoring. Our users are not functional programmers so this causes confusion. This will change the computed attribute to {} which makes more sense to the user. [AERIE-2041]
Slight refactor.
goetzrrGit
force-pushed
the
AERIE-2041--TS-library-Computed-attributes-UNIT-TO-EMPTY
branch
from
September 7, 2022 21:14
5addc85
to
fc43ce9
Compare
goetzrrGit
deleted the
AERIE-2041--TS-library-Computed-attributes-UNIT-TO-EMPTY
branch
September 7, 2022 21:27
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.
Description
In the mission model, the activity's effect model that doesn't return a computed attribute (aka void run() method of the activity) uses UNIT which represents a non-null empty value. This is used internally by AERIE
https://en.wikipedia.org/wiki/Unit_type
This UNIT value will be exposed to our users when command authoring. Our users are not functional programmers so this causes confusion.
This will change the computed attribute to
{}
which makes more sense to the user.Verification
Ran the command expansion test locally without any issues
Documentation
None
Future work
None