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

APP-6993: Deprecate legacy auth-app commands in favor of oauth-app commands #4732

Merged
merged 2 commits into from
Jan 24, 2025
Merged

APP-6993: Deprecate legacy auth-app commands in favor of oauth-app commands #4732

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54715d5
APP-6993: Deprecate legacy auth-app commands in favor of oauth-app co…
jr22 Jan 21, 2025
c149e92
lint
jr22 Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 0 additions & 100 deletions cli/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2755,106 +2755,6 @@ This won't work unless you have an existing installation of our GitHub app on yo
},
},
},
{
Name: "auth-app",
Copy link
Member Author

@jr22 jr22 Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Question: anything special i should do when removing this command? is this an ok breaking change? dont think anyone was using these since they werent feature complete. ill make sure docs are updated too

Copy link
Member

Choose a reason for hiding this comment

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

dont think anyone was using these since they feature complete.

I'm not sure I understand this sentence, do you mind rephrasing?

Re: the breaking change, I guess I want to understand a little bit more what the impetus for this is. It looks to me like the proto AuthApp methods still exist... is there a plan to delete those entirely? If not, how do we intend for them to still be used, and why are we wanting to delete them from the CLI but not elsewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

this is part of a series of PRs to deprecate these protos and CLI commands:

Ill add these links to the PR description.

As for why we're doing this... we created these commands initially almost as a quick MVP for people creating auth apps. After we created them, we realized they were missing quite a few features for people to really be able to use them to effectively create new auth applications. We recently completed a new project to add those missing features and as part of that we created other commands that duplicated this functionality (oauth-app create etc). We created duplicate commands because we realized the features belonged in a different gRPC service and needed wanted more specific CLI commands.

Copy link
Member

Choose a reason for hiding this comment

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

Got it, thanks for clarifying! In that case, this looks reasonable to me :)

Usage: "manage third party auth applications",
UsageText: createUsageText("auth-app", nil, false, true),
HideHelpCommand: true,
Subcommands: []*cli.Command{
{
Name: "register",
Usage: "register a third party auth application",
UsageText: createUsageText("auth-app register",
[]string{
generalFlagOrgID, authApplicationFlagName, authApplicationFlagOriginURIs,
authApplicationFlagRedirectURIs, authApplicationFlagLogoutURI,
}, false, false),
Flags: []cli.Flag{
&cli.StringFlag{
Name: generalFlagOrgID,
Usage: "organization ID that will be tied to auth application",
Required: true,
},
&cli.StringFlag{
Name: authApplicationFlagName,
Usage: "name for the auth application",
Required: true,
},
&cli.StringSliceFlag{
Name: authApplicationFlagOriginURIs,
Usage: "origin uris for the auth application",
Required: true,
},
&cli.StringSliceFlag{
Name: authApplicationFlagRedirectURIs,
Usage: "redirect uris for the auth application",
Required: true,
},
&cli.StringFlag{
Name: authApplicationFlagLogoutURI,
Usage: "logout uri for the auth application",
Required: true,
},
},
Action: createCommandWithT[registerAuthApplicationArgs](RegisterAuthApplicationAction),
},
{
Name: "update",
Usage: "update a third party auth application",
UsageText: createUsageText(
"auth-app update", []string{generalFlagOrgID, authApplicationFlagApplicationID, authApplicationFlagName}, false, false,
),
Flags: []cli.Flag{
&cli.StringFlag{
Name: generalFlagOrgID,
Required: true,
Usage: "organization ID that will be tied to auth application",
},
&cli.StringFlag{
Name: authApplicationFlagApplicationID,
Usage: "id for the auth application",
Required: true,
},
&cli.StringFlag{
Name: authApplicationFlagName,
Usage: "updated name for the auth application",
Required: true,
},
&cli.StringSliceFlag{
Name: authApplicationFlagOriginURIs,
Usage: "updated origin uris for the auth application",
},
&cli.StringSliceFlag{
Name: authApplicationFlagRedirectURIs,
Usage: "updated redirect uris for the auth application",
},
&cli.StringFlag{
Name: authApplicationFlagLogoutURI,
Usage: "updated logout uri for the auth application",
},
},
Action: createCommandWithT[updateAuthApplicationArgs](UpdateAuthApplicationAction),
},
{
Name: "get",
Usage: "get configuration for a third party auth application",
UsageText: createUsageText("auth-app get", []string{generalFlagOrgID, authApplicationFlagApplicationID}, false, false),
Flags: []cli.Flag{
&cli.StringFlag{
Name: generalFlagOrgID,
Required: true,
Usage: "organization ID that will be tied to auth application",
},
&cli.StringFlag{
Name: authApplicationFlagApplicationID,
Usage: "id for the auth application",
Required: true,
},
},
Action: createCommandWithT[getAuthApplicationArgs](GetAuthApplicationAction),
},
},
},
{
Name: "version",
Usage: "print version info for this program",
Expand Down
155 changes: 0 additions & 155 deletions cli/auth_application.go
View file
Open in desktop

This file was deleted.

114 changes: 0 additions & 114 deletions cli/auth_application_test.go
View file
Open in desktop

This file was deleted.

Loading