-
Notifications
You must be signed in to change notification settings - Fork 68
feat(domain discovery): add support for Domain Discovery commands #1482
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Thanks @FarhanSajid1
Just a couple of minor/nit comments and suggestions (approving to unblock).
pkg/commands/domainv1/tools/root.go
Outdated
func NewRootCommand(parent argparser.Registerer, g *global.Data) *RootCommand { | ||
var c RootCommand | ||
c.Globals = g | ||
c.CmdClause = parent.Command(CommandName, "Tools for interacting with Domains") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This description seems to general/vague to me, and suggests that it might hold commands for managing domains.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated, let me know if that makes sense
// Optional. | ||
cmd.RegisterFlagBool(cmd.JSONFlag()) | ||
cmd.CmdClause.Flag("scope", "Scope determines the availability check to perform, specify `estimate` for an estimated check").Action(cmd.scope.Set).StringVar(&cmd.scope.Value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@case-fastly should we expose this flag in the CLI? Not sure that's necessary.
WantOutput: `Domain Subdomain Zone Path | ||
fastlytest.ing fastlytest. ing | ||
fastlytesti.ng fastlytesti. ng | ||
fastlytesting.com fastlytesting. com | ||
fastlytesting.net fastlytesting. net | ||
fastlytest.in fastlytest. in /g |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@case-fastly is this output satisfactory?
All Submissions:
New Feature Submissions:
This PR adds commands for
tools
and adomain
subcommand. Thetools
command will be used for placing tools moving forward.Once a user has enabled Domain Discovery on their accounts, they should have access to the Domain Discovery endpoints.
The commands make use of the
status
andsuggest
implementation which were added in fastly/go-fastly#672.The core changes:
domains
, which will be a child oftools
.suggest
andstatus
subcommands under toolssuggest
andstatus
subcommandsChanges to Core Features:
User Impact
Users that have enabled domain discovery on their accounts can use the
suggest
andstatus
commands.Usage:
Fastly Tools
Fastly Domain Tools
Status Usage:
Status Commands
Suggest Usage:
Suggest Commands
Are there any considerations that need to be addressed for release?
No breaking changes