-
Notifications
You must be signed in to change notification settings - Fork 27
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
Parse tome paramdefs on startup #298
Parse tome paramdefs on startup #298
Conversation
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.
Looks good, glad that static hello world is no longer there
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.
What did meowware ever do to you smh
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.
Broke my heart 💔
We can make the paramdefs be more readable in the yaml, e.g. '''
|
I can play with it more but when I tried that ^ it doesn't parse right since I think everything is expecting a JSON string. |
Codecov Report
@@ Coverage Diff @@
## main #298 +/- ##
==========================================
- Coverage 74.84% 74.80% -0.05%
==========================================
Files 94 94
Lines 6106 6131 +25
==========================================
+ Hits 4570 4586 +16
- Misses 1450 1458 +8
- Partials 86 87 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Actually I think we should prioritize consistency between the standard tomes and uploaded tomes. If we want to re-do paramDefs to not be a JSON string we can do that later but for now I think we should stick with the agreed on paramDefs format. |
Gonna pass to @cmp5987 parameters: Some(r#"{"cmd":"echo main_loop_test_success"}"#.to_string()) We need to add the "parameters" field to the formatVariables in the @cmp5987 pointed out this is confusing especially since the defs follow the pattern: The params, and paramdefs work flow should probably be re-worked at a later date so we're not using JSON strings at all. |
|
|
a1ba38e
to
700fccb
Compare
…ttps://github.com/KCarretto/realm into 295-creating-new-tasks-doesnt-prompt-for-parameters
Parse tome paramdefs on startup (#298) * Set paramdefs on parse. * Removes debug statements. * Should fix tests * Missed this one * Updated UI. * Yay this works. * Remove debug statements. * Remove debug. * Rebuild UI. * auto reformat paramdefs to a JSON string. * Increase param def string size. * Updated UI * Add a non param example. * Add default value to params * Remove test. * Rebuild ui. * Remove test tome. * Add test 3 * Remove example 3 * Try updating parameters too. * Manually generated * Switch back to entry name metadata name is complex * Increase quest output drawer size (#306) * Set paramdefs on parse. * Removes debug statements. * Should fix tests * Missed this one * Updated UI. * Yay this works. * Remove debug statements. * Remove debug. * Rebuild UI. * auto reformat paramdefs to a JSON string. * Increase param def string size. * Updated UI * Add a non param example. * Add default value to params * Remove test. * Rebuild ui. * Remove test tome. * Add test 3 * Remove example 3 * Try updating parameters too. * Manually generated * Switch back to entry name metadata name is complex * Update UI.
What type of PR is this?:
/kind bug
What this PR does / why we need it:
SetParamDefs
during tome parsingWhich issue(s) this PR fixes:
Fixes #295