tbouron commented on issue #129: UI for defining parameters
URL: https://github.com/apache/brooklyn-ui/pull/129#issuecomment-478991360
 
 
   >yeah, i buy that. i don't see an easy way to do it with validation for the 
case where duplicate keys are set in yaml. (we already did it if you tried to 
supply duplicate keys in the ui so i think from yaml is the only way.) what 
i've done instead is that on model load if we find a duplicate we append a 
number to it, so brooklyn.parameters: [ { name: y, type: string }, { name: y, 
type: string }, { name: y, type: string } ] becomes y y2 y3, with warnings 
logged to the console. it's awkward and surprising but at least doesn't break 
the ui. we could have yaml model validation errors as well for this though the 
problem would still manifest in the graphical composer if invalid yaml was 
passed in so we need a way to handle it here. the other option would be to 
refuse to show parameters.
   
   Sounds good for now 👍 
   
   > if it's valid JSON it works, but it must be e.g. ["required"] -- -have 
changed message to note it needs the brackets and double quotes. could improve 
by supporting yaml parsing or a dedicated editor but going for minimal working 
capability here. also agree clearing on unfocus is awkward but of course 
there's a lot of work to store invalid models if we did something else. we 
could maybe forbid the unfocus.
   
   Aaaah it requires JSON! Great for the error message, **it would be also 
great to specify this in the information popup for the constraints @ahgittin.**
   
   > doesn't moot mean "no longer relevant or not worth discussion"? i think 
you mean it's a step backwards. personally i thought having two was awkward and 
one is fairly self-explanatory. happy to see further evolution and testing but 
agree it's such a minor point as to be moot :).
   
   Probably my flawed French translation: `moot` means `subject to debate, 
arguable` to me. 
   
   > i removed checkboxes so in total we have a real estate savings. i think 
icons make it look more polished, but agree the x is confusing. have changed it 
to a stop-circle (bit white square in black circle). again happy to see further 
evolution but i'd like to keep the icons. if you feel strongly let's get a 
tie-break opinion!
   
   Great!
   
   Happy to merge once we have the explanation for the JSON constraints 
@ahgittin !

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to