Skip to content

Remove DEPRECATED text, since it's just the default#9231

Merged
ndeloof merged 1 commit intodocker:v2from
ulyssessouza:fix-text-flags
Mar 11, 2022
Merged

Remove DEPRECATED text, since it's just the default#9231
ndeloof merged 1 commit intodocker:v2from
ulyssessouza:fix-text-flags

Conversation

@ulyssessouza
Copy link
Contributor

What I did
Remove DEPRECATED text, since it's just the default

Related issue
#9229 (comment)

(not mandatory) A picture of a cute animal, if possible in relation with what you did

@ulyssessouza ulyssessouza requested review from glours and ndeloof March 4, 2022 16:53
Copy link
Contributor

@ndeloof ndeloof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

strictly speaking, we should handle --interactive=false and not just make this flag a No-Op, but I guess that's a corner case :D

@thaJeztah
Copy link
Member

Needs a rebase, @ulyssessouza

Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
@ndeloof ndeloof enabled auto-merge (rebase) March 11, 2022 12:01
@ndeloof ndeloof merged commit 6ce57ea into docker:v2 Mar 11, 2022
@thaJeztah
Copy link
Member

strictly speaking, we should handle --interactive=false and not just make this flag a No-Op, but I guess that's a corner

Yes, would be good to support that as well; also (from #9207 (comment)) probably produce an error when passing both -t or -i and -T (conflicting options)

@ndeloof
Copy link
Contributor

ndeloof commented Mar 11, 2022

Already added support for --interactive=false on https://github.com/docker/compose/blob/v2/cmd/compose/run.go#L223
Support for combinations of --tty=false and --no-tty is a bit more complex, will look into this on my spare time :)

@thaJeztah
Copy link
Member

Yes, that one is more tricky; if the options do not "conflict", we could accept them, but if they "conflict", it's more tricky to define the behaviour (which one should take precedence??). In such cases, I usually make that an error-condition; the advantage with that is that you're sure nobody is able to use the combination (as it errors), which leaves the ability to "allow" the combination in future (if there's a decision on what the expected outcome is).

@ndeloof
Copy link
Contributor

ndeloof commented Mar 11, 2022

indeed. I planned to rely on flags.Changed to check is tty is true because this is default or user explicitly user -t, so that we can detect conflict with -T

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants