Skip to content
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

update docs with currently supported features options #1314

Merged
merged 1 commit into from
Sep 6, 2018

Conversation

AntaresS
Copy link
Contributor

- What I did

  • add a section of currently supported features options.

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Anda Xu [email protected]

@AntaresS
Copy link
Contributor Author

cc @andrewhsu

@codecov-io
Copy link

codecov-io commented Aug 27, 2018

Codecov Report

Merging #1314 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1314   +/-   ##
=======================================
  Coverage   54.73%   54.73%           
=======================================
  Files         292      292           
  Lines       19267    19267           
=======================================
  Hits        10545    10545           
  Misses       8063     8063           
  Partials      659      659

`buildkit` as the default docker image builder.

The list of currently supported feature options:
- `buildkit`: It enables `buildkit` as default builder when set to `true` or disable it by
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo: s/disable/disables/

@@ -1423,6 +1423,15 @@ This is a full example of the allowed configuration options on Windows:
"insecure-registries": []
}
```
#### Feature options
Copy link
Member

Choose a reason for hiding this comment

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

Can you add an empty line before this header? Perhaps not needed, but I've seen some markdown processors liking to have that 😅


The list of currently supported feature options:
- `buildkit`: It enables `buildkit` as default builder when set to `true` or disable it by
`false`. Note that if this option is not explicitly set in the daemon config file, then it
Copy link
Member

Choose a reason for hiding this comment

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

We generally reserve "Note" for a "Note" block. This could be written as;

If this option is not set in the daemon configuration file, then it is up to the cli
to specify which builder to invoke.

Copy link
Member

Choose a reason for hiding this comment

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

Just a thought; perhaps we should describe a bit more what happens when set to false (i.e., BuildKit is disabled, and cannot be invoked by the CLI; the "classic" builder is used instead. Do we have a standard name for the classic builder? (legacy builder? v1 builder?))

@@ -1423,6 +1423,15 @@ This is a full example of the allowed configuration options on Windows:
"insecure-registries": []
}
```
#### Feature options
The optional field `features` in `daemon.json` grants docker users more granularity to
Copy link
Member

Choose a reason for hiding this comment

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

"docker users" feels a bit strange here (it's a daemon-level option, so users <--> administrators?).

Perhaps

The optional field `features` in `daemon.json` allows you to enable or disable specific daemon features.

Copy link
Contributor

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit ce4a9f8 into docker:master Sep 6, 2018
@GordonTheTurtle GordonTheTurtle added this to the 18.09.0 milestone Sep 6, 2018
nashasha1 pushed a commit to nashasha1/cli that referenced this pull request Sep 7, 2018
update docs with currently supported features options

Signed-off-by: nashasha1 <[email protected]>
nashasha1 pushed a commit to nashasha1/cli that referenced this pull request Sep 7, 2018
update docs with currently supported features options

Signed-off-by: nashasha1 <[email protected]>
nashasha1 pushed a commit to nashasha1/cli that referenced this pull request Sep 7, 2018
update docs with currently supported features options

Signed-off-by: nashasha1 <[email protected]>
@thaJeztah thaJeztah modified the milestones: 18.09.0, 19.03.0 Sep 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants