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

fix: toggle flag aliases #269

Merged
merged 7 commits into from
May 10, 2024
Merged

fix: toggle flag aliases #269

merged 7 commits into from
May 10, 2024

Conversation

dbolson
Copy link
Contributor

@dbolson dbolson commented May 10, 2024

Adds the flags toggle-on/toggle-off actions back.

Running ldcli flags --help shows the new actions:

Available Commands:
  copy                           Copy feature flag
  create                         Create a feature flag
  create-migration-safety-issues Get migration safety issues
  delete                         Delete feature flag
  get                            Get feature flag
  get-status                     Get feature flag status
  get-status-across-environments Get flag status across environments
  list                           List feature flags
  list-expiring-context-targets  Get expiring context targets for feature flag
  list-expiring-user-targets     Get expiring user targets for feature flag
  list-statuses                  List feature flag statuses
  toggle-off                     Turn a feature flag off
  toggle-on                      Turn a feature flag on
  update                         Update feature flag
  update-expiring-targets        Update expiring context targets on feature flag
  update-expiring-user-targets   Update expiring user targets on feature flag

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

Provide links to any issues in this repository or elsewhere relating to this pull request.

Describe the solution you've provided

Provide a clear and concise description of what you expect to happen.

Describe alternatives you've considered

Provide a clear and concise description of any alternative solutions or features you've considered.

Additional context

Add any other context about the pull request here.

@@ -395,7 +395,8 @@ Global Flags:
Additional help topics:{{range .Commands}}{{if .IsAdditionalHelpTopicCommand}}
{{rpad .CommandPath .CommandPathPadding}} {{.Short}}{{end}}{{end}}{{end}}{{if .HasAvailableSubCommands}}

Use "{{.CommandPath}} [command] --help" for more information about a command.{{end}}`
Use "{{.CommandPath}} [command] --help" for more information about a command.{{end}}
`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this newline so the prompt wouldn't end up on the last line as the help output, consistent with the other inline templates.

@@ -182,6 +183,14 @@ func NewRootCommand(
cmd.AddCommand(resourcecmd.NewResourcesCmd())
resourcecmd.AddAllResourceCmds(cmd, clients.ResourcesClient, analyticsTrackerFn)

// add non-generated 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.

We can add other aliased or other commands here.


import "net/url"

type MockClient struct {
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 ended up not going with the testify mock code. If you'd prefer that way, I can change this.

@dbolson dbolson requested a review from k3llymariee May 10, 2024 22:11
Copy link
Contributor

@k3llymariee k3llymariee left a comment

Choose a reason for hiding this comment

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

just a comment about updating the usage template but that can be added in a follow-up

"ldcli/internal/resources"
)

func NewToggleOnCmd(client resources.Client) *cobra.Command {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we also need to set the usage template for these, in order to parse out the required/optional flags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Help now looks like this

Turn a feature flag on

Usage:
  ldcli flags toggle-on [flags]

Required flags:
      --environment string   The environment key
      --flag string          The feature flag key
      --project string       The project key

Optional flags:


Global Flags:
  -h, --help                  Help for this command
      --access-token string   LaunchDarkly access token with write-level access
      --analytics-opt-out     Opt out of analytics tracking
      --base-uri string       LaunchDarkly base URI (default "https://app.launchdarkly.com")
  -o, --output string         Command response output format in either JSON or plain text (default "plaintext")

@dbolson dbolson merged commit 76de2a5 into main May 10, 2024
2 checks passed
@dbolson dbolson deleted the sc-243812/toggle-flag-aliases branch May 10, 2024 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants