-
Notifications
You must be signed in to change notification settings - Fork 0
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
updates to scoped variables struct #221
Conversation
internal_api: | ||
nullable: true | ||
description: Descibes an object with variables that the internal api will serve after runtime starts |
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.
This needs updated to something like "If set, this scoped variable will be available over the internal API. Contains settings for accessing this variable over the internal API."
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.
Updated to that ^
type: object | ||
properties: | ||
duration: | ||
$ref: ../Duration.yml | ||
type: object | ||
description: Duration is a time string that the internal api will serve that variable after runtime starts |
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.
API - consistency! Also don't forget punctuation.
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.
got it
properties: | ||
decode: | ||
description: Decode tells Cycle that the file is a base64 encoded string |
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.
no need for the "Decode" at the beginning - it's obvious because of what field you're in. Also, this is missing something - specifically, Cycle will try to decode the base64 string.
"When true, Cycle will interpret this variable as a base-64 encoded string, and decode it before passing it into the container."
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.
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.
dont use example that way, just show the default without the Default path- part
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.
Ok
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.
did you update the description?
properties: | ||
url: | ||
type: string | ||
description: The URL to call to produce the value. | ||
headers: | ||
type: object | ||
description: Additional information about a container | ||
description: Additional information about a URL source |
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.
punctuation, but try to bring this inline with the other description for the source block.
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.
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.
is that what headers is? that doesn't seem right.
additionalProperties: {} | ||
auth_token_url: | ||
nullable: true | ||
type: string | ||
description: The response is from auth_token_url will be sent as header authorization which allows third party integration |
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.
This comment needs some work. The grammar is strange, and it isn't really specifying what the field is.
"An optional URL that can be provided to authenticate with a third party secret service. Cycle will make a request to this URL before fetching the secret URL, and use the response as the value of an Authorization header when requesting the secret."
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.
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.
saying nullable in the description is strange. just drop it from here
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.
Got it. You originally said optional, which wasn't true
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.
LGTM.
Update to scoped var struct