Skip to content

Move daemon-only flags into the daemon config struct#10453

Merged
jessfraz merged 1 commit intomoby:masterfrom
aidanhs:aphs-move-daemon-flags
Feb 8, 2015
Merged

Move daemon-only flags into the daemon config struct#10453
jessfraz merged 1 commit intomoby:masterfrom
aidanhs:aphs-move-daemon-flags

Conversation

@aidanhs
Copy link
Copy Markdown
Contributor

@aidanhs aidanhs commented Jan 30, 2015

No description provided.

@jessfraz
Copy link
Copy Markdown
Contributor

Can you please sign your commits following these rules:

https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work

The easiest way to do this is to amend the last commit:

git commit --amend -s --no-edit && git push -f

@aidanhs aidanhs force-pushed the aphs-move-daemon-flags branch 2 times, most recently from 99ee814 to 73a0adc Compare January 30, 2015 03:04
@aidanhs
Copy link
Copy Markdown
Contributor Author

aidanhs commented Jan 30, 2015

Done and also fixed some gofmt issues.

The flag ordering seems to be completely arbitrary so I've just inserted them in a random place.

docker/flags.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No, these can be used elsewhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Where? The daemon is responsible for creating the socket and serving the API. These arguments don't apply to any other docker subcommands...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know "docker info" at least has different behavior with "-D" passed to
either the client or the daemon.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Which is exactly why I left "-D" alone.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

(the only things changed are -G and --api-enable-cors)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍 (@jfrazelle triiicked me)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh wow read this diff wrong

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The tradeoff of enforced gofmt :)

@tiborvass
Copy link
Copy Markdown
Contributor

@aidanhs Following your last comments on #10535 (comment)

Docker maintainers (> Docker Inc), are currently overwhelmed by the 1.5 release hence our slow down in PR review. We read all comments, and we will review all PRs in due time, so please bear with us :)

Btw, your PR here seems very reasonable and we should be able to merge it reasonably quickly, so if you could rebase that'd be fantastic! Thanks for your patience!

@jessfraz
Copy link
Copy Markdown
Contributor

jessfraz commented Feb 6, 2015

needs a rebase, so sorry, if you ping me after I will review immediately

Signed-off-by: Aidan Hobson Sayers <aidanhs@cantab.net>
@aidanhs aidanhs force-pushed the aphs-move-daemon-flags branch from 73a0adc to 06ea5fe Compare February 8, 2015 12:57
@aidanhs
Copy link
Copy Markdown
Contributor Author

aidanhs commented Feb 8, 2015

@tiborvass that's fine, Docker is hardly a low activity project, I can imagine there are busy periods which is why I made my comment - no point spending time if maintainers are busy with other things :)

@jfrazelle ping

@tiborvass
Copy link
Copy Markdown
Contributor

LGTM

1 similar comment
@jessfraz
Copy link
Copy Markdown
Contributor

jessfraz commented Feb 8, 2015

LGTM

jessfraz pushed a commit that referenced this pull request Feb 8, 2015
Move daemon-only flags into the daemon config struct
@jessfraz jessfraz merged commit 04b0bf4 into moby:master Feb 8, 2015
@aidanhs aidanhs deleted the aphs-move-daemon-flags branch February 27, 2015 03:13
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.

4 participants