-
Notifications
You must be signed in to change notification settings - Fork 2
feat(commands): implement credit/org details commands #16
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: release/1.0
Are you sure you want to change the base?
Conversation
7ff43fe to
3eaa8d4
Compare
3eaa8d4 to
0d3856b
Compare
0d3856b to
479f501
Compare
| "github.com/censys/cencli/internal/pkg/domain/responsemeta" | ||
| ) | ||
|
|
||
| //go:generate mockgen -destination=../../../gen/app/organizations/mocks/organizationservice_mock.go -package=mocks -mock_names Service=MockOrganizationsService . Service |
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.
I didnt realize we were doing this. Nice!
| func (c *Command) Examples() []string { | ||
| return []string{ | ||
| "# Show credits for your stored organization", | ||
| "--org-id <uuid> # Show credits for a specific organization", |
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.
Im not sure it makes sense for us to have org id as an argument. The expected flow would be to select a different active org in my mind.
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.
I kinda agree, but to be consistent with our other commands, we allow users to override the org they want to use, and this also supports users who don't want to configure their org beforehand
| type noOrgIDError struct{} | ||
|
|
||
| func (e *noOrgIDError) Error() string { | ||
| return "no organization ID configured. Use --org-id flag or run 'censys config org-id set <org-id>' to set a default" |
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.
Eh I guess its alright as an alternative approach. Doesn't cost us anything.
aspacewalz
left a comment
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.
LGTM!
I'm not sold on the need for the org-id argument. It might cause confusion and its more difficult to remove once added, but it also doesn't cost us anything to support it. Im ok with leaving it, but wanted to give my thoughts.
No description provided.