Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Fix the -set-default flag for context create. #3044

Merged
merged 3 commits into from
Feb 28, 2022

Conversation

izaaklauer
Copy link
Contributor

The flag was unused before this point. This also sets the default state to "true", which is what I think most users would expect.

How to verify

$ waypoint context create is-default
$ waypoint context list
    |         NAME         |  PLATFORM  |                                    SERVER ADDRESS
----+----------------------+------------+----------------------------------------------------------------------------------------
  * | is-default           | n/a        |

$ waypoint context create -set-default=false not-default

Context "not-default" created.
$ waypoint context list
    |         NAME         |  PLATFORM  |                                    SERVER ADDRESS
----+----------------------+------------+----------------------------------------------------------------------------------------
  * | is-default           | n/a        |
    | not-default          | n/a        |

THe flag was unused before this point. This also sets the default state to "true", which is what I think most users would expect.

```
$ waypoint context list
    |         NAME         |  PLATFORM  |                                    SERVER ADDRESS
----+----------------------+------------+----------------------------------------------------------------------------------------
  * | is-default           | n/a        |

izaak ~ $ ~/dev/waypoint/waypoint context create -set-default=false not-default

Context "not-default" created.
$ waypoint context list
    |         NAME         |  PLATFORM  |                                    SERVER ADDRESS
----+----------------------+------------+----------------------------------------------------------------------------------------
  * | is-default           | n/a        |
    | not-default          | n/a        |
```
@github-actions github-actions bot added the core label Feb 25, 2022
@izaaklauer izaaklauer marked this pull request as ready for review February 25, 2022 20:40
@izaaklauer izaaklauer requested review from a team and jgwhite February 25, 2022 20:40
Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

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

Name: "set-default",
Target: &c.flagSetDefault,
Default: true,
Usage: "Set this context as the new default for the CLI. Defaults to true.",
Copy link
Member

Choose a reason for hiding this comment

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

BTW we don't need the Defaults to true in the doc string since the CLI adds this automatically:

brian@localghost:waypoint λ waypoint context create -help                                                                                                                                                                                                                                                             
Command Options:
...
...

  -set-default
      Set this context as the new default for the CLI. Defaults to true. The
      default is true.

Copy link
Member

Choose a reason for hiding this comment

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

I can make a PR to fix this real quick

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh ha, I was fixing the website docs. Looks like the CLI prints the default, but the website docs don't?

Copy link
Member

Choose a reason for hiding this comment

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

Noooo! I see, that's weird. 🤔 Maybe we need to update our docs gen then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree that the cli help text is more important than the website docs, so I support reverting this for now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants