Skip to content
This repository was archived by the owner on Sep 3, 2025. It is now read-only.

Share ~/.ssh/config with containers#36

Merged
slucero merged 3 commits intomasterfrom
ssh-config-mount
Sep 25, 2019
Merged

Share ~/.ssh/config with containers#36
slucero merged 3 commits intomasterfrom
ssh-config-mount

Conversation

@gustavderdrache
Copy link
Copy Markdown
Contributor

This allows containers to see a user's SSH config (when present). The most important effect of this change is that it will allow users to use their jump box settings without having to re-enter them into their Drush aliases.

Copy link
Copy Markdown
Contributor

@slucero slucero left a comment

Choose a reason for hiding this comment

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

Can we add the basic doc blocks and function descriptions for the add functions?

  • addSshConfigFile()
  • addKnownHostsFile()

@gustavderdrache
Copy link
Copy Markdown
Contributor Author

Added doc comments and clarified the return value of addWellKnownFile().

import addWellKnownFile from './addWellKnownFile';

/**
* Returns the `docker run` flags necessary to share the user's SSH configuration file.
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.

I think the description for these functions was interchanged and should be:

 * Returns the `docker run` flags necessary to share the user's known_hosts file.

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.

Suggested change
* Returns the `docker run` flags necessary to share the user's SSH configuration file.
* Returns the `docker run` flags necessary to share the user's known_hosts file.

Comment thread src/docker/getSshOptions/addSshConfigFile.ts Outdated
@gustavderdrache
Copy link
Copy Markdown
Contributor Author

Whoops. Function comments should be the right way 'round now.

@slucero slucero merged commit fcd2988 into master Sep 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants