Add docker daemon command instead of -d #6055
Conversation
|
This solution seems very error prone to me. |
|
What should happen when you specify an option to be a default one, like |
|
tianon I agree which is why I suggested we drop -d and force the command to be daemon docker daemon [OPTIONS] Then you would not allow options before the command.
All I can check now is if the default changed, which tells me they used the option. |
|
I think we should do multiple parse. One to parse only |
|
@vieux that sounds like an excellent halfway point while we wait for (which, last time we discussed, was expected to come as soon as upstream Go managed to get us a reliable method of actual daemonization/backgrounding) |
|
But that is just a complication of not using the command daemon. We have a few weeks to simpify the code, then you are stuck with it forever. |
|
Why can't we do "docker daemon" now? |
|
+1 for |
|
I changed the title of this PR |
|
I have updated this pull request to substitute -d and --daemon with daemon command before flag.Parse() This means existing installs will continue to work, but print a log message if they use -d or --daemon. |
|
@rhatdan: can you put a deprecated notice, so that we can remove any mention of |
|
I can change the log message to say that -d/--daemon is depracated and will be removed in a future release. |
|
+1 to the change, however, you still need to update |
|
Most of those distro pings are directly in |
|
Fixed up contrib/init and cli.md |
|
👍 contrib LGTM |
|
Docs LGTM, 🎉 |
There was a problem hiding this comment.
The Docker daemon is usually...
|
Comments on commit. |
|
@crosbymichael @vieux do you guys want this? I do. |
|
Not really, I'm not against as long as we keep |
|
Makes a lot of sense to me. It clearly separates out the concerns of client and server. |
|
I can work it again, if it has a chance of upstreaming. |
QE Reports that you can put lots of bogus options on the cli before the command which are ignored if you specify a command. We should report a usage error if a user does this. I believe a better long term solution would be to remove the -d options and add a daemon command. This would make the handling of options the same for all use cases. This patch will still handle the -d option by substituting "daemon" command and print a warning message to the user. Docker-DCO-1.1-Signed-off-by: Dan Walsh <dwalsh@redhat.com> (github: rhatdan)
|
I would loooove to deprecate |
|
I have asked a couple of new guys coming to my team to look into it, unless someone else is working on it. I took a quick look and my original patches are way out of data at this point. Although all of the docs stuff is still good. :^( |
|
i am seeing merge conflicts in the code? |
|
Yes this patch set needs to be rewritten. The patch no longer applies. I don't have time to rework it now, but am trying to get others to do it. If you want to take a stab, go for it. |
|
Ok, since this is not able to be merged and it's not exactly clear whether this is something that we should have in docker it maybe better to discuss this in an issue before rebasing and rewriting this PR. Sorry for the long response. |
|
Well I think it is something we should definitely be in docker. I just don't have the time to rewrite the patch set. The current split causes man pages and option handling to be buggy and not easy to understand when to use certain options. IE A lot of options are only valid for use with the -d option. Changing this to daemon would clear this up. |
QE Reports that you can put lots of bogus options on the cli before the command
which are ignored if you specify a command. We should report a usage error
if a user does this.
I believe a better long term solution would be to remove the -d options and add a daemon command. This would make the handling of options the same for all use
cases.
Docker-DCO-1.1-Signed-off-by: Dan Walsh dwalsh@redhat.com (github: rhatdan)