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.

Sign up for GitHub

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

Jump to bottom

feat: add docker context module #996

Merged
merged 11 commits into from Apr 5, 2020
Merged

feat: add docker context module #996

merged 11 commits into from Apr 5, 2020

Conversation

Southclaws
Copy link
Contributor

@Southclaws Southclaws commented Mar 11, 2020 •
edited

Description

Adds a simple module that checks for a Docker config file and if present, reads the currentContext value out and displays on the prompt with a whale.

Motivation and Context

Closes #995

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

  • I have tested using MacOS
  • I have tested using Linux
  • I have tested using Windows

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@Southclaws
feat: add docker context module
5ea85f9 
Adds a simple module that checks for a Docker config file and if present, reads the `currentContext` value out and displays on the prompt with a whale.
@andytom
Copy link
Member

andytom commented Mar 18, 2020

Thanks for the contribution @Southclaws.
Would it make sense to check the current working directory and enable the module if it contains a docker or docker compose file?

@Southclaws
Copy link
Contributor Author

That's probably worth adding an option for as it's not something I'd use - I often run docker commands outside a directory with a compose file in (non-compose things like docker ps etc)

@Southclaws
feat: add only_with_compose_yml option to docker_context
dda4f43 
When enabled, will only show the docker context name if there's a docker-compose.yml file in the current directory.
@Southclaws
Copy link
Contributor Author

Done, however I can't compile or test because Cargo/crates.io has just completely stopped working.

andytom
andytom reviewed Mar 29, 2020
View reviewed changes
Copy link
Member

@andytom andytom left a comment

Choose a reason for hiding this comment

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

I have added a few suggestions but in general this LGTM.

src/modules/docker_context.rs Outdated Show resolved Hide resolved
src/modules/docker_context.rs Outdated Show resolved Hide resolved
src/modules/docker_context.rs Outdated Show resolved Hide resolved
docs/config/README.md Outdated Show resolved Hide resolved
@Southclaws @andytom
Update src/modules/docker_context.rs
d657552 
Co-Authored-By: Thomas O'Donnell <andytom@users.noreply.github.com>
@Southclaws
Copy link
Contributor Author

Ah, yes, sorry I completely forgot about this PR! Will clean up today 👍

andytom
andytom reviewed Apr 4, 2020
View reviewed changes
Copy link
Member

@andytom andytom left a comment

Choose a reason for hiding this comment

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

@Southclaws, sorry for the delay in reviewing this, I've added a suggested change to the README.md. Also as noted in the comments what do you think about enabling the module by default but only when there are the Docker files.

docs/config/README.md Outdated Show resolved Hide resolved
src/configs/docker_context.rs Outdated Show resolved Hide resolved
Southclaws and others added 2 commits April 5, 2020 20:19
@Southclaws @andytom
Update docs/config/README.md
48f301b 
Co-Authored-By: Thomas O'Donnell <andytom@users.noreply.github.com>
@Southclaws @andytom
Update src/configs/docker_context.rs
8b649ce 
Co-Authored-By: Thomas O'Donnell <andytom@users.noreply.github.com>
@Southclaws
Copy link
Contributor Author

Thanks! Those suggestions made sense. LTGM! 👍

@andytom andytom merged commit b685bb2 into starship:master Apr 5, 2020
dagbrown pushed a commit to dagbrown/starship that referenced this pull request Oct 22, 2021
@Southclaws @andytom @dagbrown
feat: add docker context module (starship#996)
661505e 
* feat: add docker context module

Adds a simple module that checks for a Docker config file and if present, reads the `currentContext` value out and displays on the prompt with a whale.

* feat: add `only_with_compose_yml` option to docker_context

When enabled, will only show the docker context name if there's a docker-compose.yml file in the current directory.

* Update src/modules/docker_context.rs

Co-Authored-By: Thomas O'Donnell <andytom@users.noreply.github.com>

* Update src/modules/docker_context.rs

Co-Authored-By: Thomas O'Donnell <andytom@users.noreply.github.com>

* rename `only_with_compose_yml` configuration key to a more generic `only_with_files`

* Update src/modules/docker_context.rs

Co-Authored-By: Thomas O'Donnell <andytom@users.noreply.github.com>

* re-order configuration table

* Update docs/config/README.md

Co-Authored-By: Thomas O'Donnell <andytom@users.noreply.github.com>

* Update src/configs/docker_context.rs

Co-Authored-By: Thomas O'Donnell <andytom@users.noreply.github.com>

Co-authored-by: Thomas O'Donnell <andytom@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andytom andytom andytom approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Display Current Docker Context
2 participants
@Southclaws @andytom