Uh oh!
There was an error while loading. Please reload this page.
config: Explicily make consoleSize unspecified if terminal is false or unset - #863
Merged
crosbymichael merged 1 commit intoJun 1, 2017
Merged
Conversation
wking
commented
May 31, 2017
ContributorAuthor
I don't think the old language is clear enough for me to make a call about whether this is a breaking change (and therefore should happen before 1.0) or not. If maintainers feel like the old language is clear enough to make that call, they can probably just close this with a “this is already clear enough” comment (although I'd personally like them to also include a longer description about what the old language means, since it's not clear to me). If maintainers agree that the old language is unclear, then I think we want to land something to clear this up before 1.0, whether it's this PR or a maintainer-authored PR. |
wkingforce-pushed
the
console-size-if-attached
branch
from
June 1, 2017 15:35
f4255fb to
29074b4Comparewking
commented
Jun 1, 2017
ContributorAuthor
crosbymichael
requested changes
Jun 1, 2017
| As an example, if set to true on Linux a pseudoterminal pair is allocated for the container process and the pseudoterminal slave is duplicated on the container process's [standard streams][stdin.3]. | ||
| * **`consoleSize`** (object, OPTIONAL) specifies the console size in characters of the terminal if attached, containing the following properties: | ||
| * **`consoleSize`** (object, OPTIONAL) specifies the console size in characters of the terminal. | ||
| `consoleSize` is unspecified if `terminal` is `false` or unset. |
Member
There was a problem hiding this comment.
Please change unspecified to ignored
ContributorAuthor
…r unset The old language is from a502caf (config: Add consoleSize to process, 2016-09-14, opencontainers#563), where nobody commented on the "if attached" wording [1]. But reading the old line now, it's not clear to me what consoleSize means when terminal is not true. This commit explicitly declares consoleSize ignored in that condition. I'd rather have made it unspecified, so runtimes are free to do what they want short of erroring out, but Michael wanted the more specific "ignored" [2]. I considered making the property undefined or requiring it to be unset, but those seemed too strict given our permissive "MUST ignore unknown properties" extensibility requirement. [1]: opencontainers#563 [2]: opencontainers#863 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
wkingforce-pushed
the
console-size-if-attached
branch
from
June 1, 2017 21:21
29074b4 to
ed6c9efCompareContributor
LGTM |
crosbymichael
approved these changes
Jun 1, 2017
Member
LGTM |
Merged
This file contains hidden or 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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
The old language is from #563, where nobody commented on the “if attached” wording. But reading the old line now, it's not clear to me what
consoleSizemeans whenterminalis nottrue.This commit explicitly declares
consoleSizeunspecified in that condition, so runtimes are free to do what they want short of erroring out. I considered making the property undefined or requiring it to be unset, but those seemed too strict given our permissive “MUST ignore unknown properties” extensibility requirement.