Skip to content

Conversation

@thomas-riccardi
Copy link
Contributor

closes #1413

- What I did
ddadd3d mass standardized the
formatting, with some errors.

This commit fixes errors on login.md

  • revert wrong Logging out headline
  • restore correct level for some headlines (relative to parent
    headline level change)
  • re-add Usage

- How I did it
.

- How to verify it
Check rendered documentation

- Description for the changelog
Better login documentation layout

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.

thank you! I left some thoughts; happy to discuss 🤗

- Microsoft Windows Credential Manager: https://github.com/docker/docker-credential-helpers/releases
- [pass](https://www.passwordstore.org/): https://github.com/docker/docker-credential-helpers/releases

#### Usage
Copy link
Member

Choose a reason for hiding this comment

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

Thinking if we should use a more descriptive title; perhaps "Credentials store configuration", or "Configure the credentials store" ?

(open to suggestions 😅)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went for the Configure the ... way. Both ways already existed:

run.md:787:### Configure namespaced kernel parameters (sysctls) at runtime
pull.md:40:### Proxy configuration

operations concerning credentials of the specified registries.

### Logging out
#### Usage
Copy link
Member

Choose a reason for hiding this comment

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

Thinking if we should use a more descriptive title; perhaps "Credential helper configuration", or "Configure credential helpers" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

credential store (`credsStore` or the config file itself) will not be used for
operations concerning credentials of the specified registries.

### Logging out
Copy link
Member

Choose a reason for hiding this comment

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

Good catch; would be worth to add a "Related commands" at the end of this file, to link to logout (similar to https://github.com/docker/cli/blob/master/docs/reference/commandline/plugin_create.md#related-commands)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

#### Usage

If you are currently logged in, run `docker logout` to remove
the credentials from the default store.
Copy link
Member

Choose a reason for hiding this comment

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

If you're interested in making some additional changes (could be in a follow-up); I think we should include a link somewhere to the section describing where the cli configuration file is; https://github.com/docker/cli/blob/master/docs/reference/commandline/cli.md#configuration-files)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll probably do that in a follow-up PR soon.

ddadd3d mass standardized the
formatting, with some errors.

This commit fixes errors on `login.md`:
- revert wrong `Logging out` headline
- restore correct level for some headlines (relative to parent
  headline level change)
- re-add `Usage` headlines, with better name

Also add `related commands` headline on `login` and `logout`.

Signed-off-by: Thomas Riccardi <thomas@deepomatic.com>
@codecov-io
Copy link

Codecov Report

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

@@           Coverage Diff           @@
##           master    #1414   +/-   ##
=======================================
  Coverage   54.26%   54.26%           
=======================================
  Files         289      289           
  Lines       19331    19331           
=======================================
  Hits        10490    10490           
  Misses       8165     8165           
  Partials      676      676

Copy link
Collaborator

@albers albers left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

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, thanks!!

@thaJeztah thaJeztah merged commit 254bcd2 into docker:master Oct 8, 2018
@GordonTheTurtle GordonTheTurtle added this to the 19.03.0 milestone Oct 8, 2018
@thomas-riccardi thomas-riccardi deleted the 1413-fix-login-documentation-layout branch October 8, 2018 14:52
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.

login documentation strange headlines

5 participants