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

Ev/tctl #1248

Merged
merged 2 commits into from
Sep 4, 2017
Merged

Ev/tctl #1248

merged 2 commits into from
Sep 4, 2017

Conversation

kontsevoy
Copy link
Contributor

@kontsevoy kontsevoy commented Sep 4, 2017

tctl changes (polish for 2.3), refs #1137

  • tctl get user/joe now works (as reported in tctl <resource command> user #1247)
  • tctl create/rm roles changes (moved to 'e')
  • tctl users ls works differently for OSS-vs-Enterprise (and matches the docs again)
  • added synonyms for various resources
  • made YAML the default output for tctl get
  • added better help + examples for tctl get
  • edited error messages
  • added the system of "command plugins" which allows 'e' sub-repo of tctl to modify default behavior.
  • minor refactoring

Related PR

@kontsevoy kontsevoy added this to the 2.3 milestone Sep 4, 2017
@kontsevoy kontsevoy force-pushed the ev/tctl branch 3 times, most recently from d033e44 to 7178cb7 Compare September 4, 2017 02:23
This commit refs #1137

- tctl get user/joe now works (as reported in #1247)
- tctl create/rm roles changes
- added synonyms for various resources
- made YAML the default output for tctl get
- added better help + examples for tctl get
- edited error messages
- minor refactoring
- added the system of "command plugins" which allows enterprise version
  of tctl to introduce different behavior to OSS commands
@@ -215,33 +198,38 @@ func (u *ResourceCommand) Create(client *auth.TunClient) error {
case "":
return trace.BadParameter("missing resource kind")
default:
return trace.BadParameter("%q is not supported", raw.Kind)
// customized creation:
if u.Impl != nil && u.Impl.Create != nil {
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 using interfaces would be more elegant here as you won't need to check this for Nil, simply provide default interface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but how would you customize just one method? interface would force to you create a bunch of do-nothing stubs with "ErrorNotImpl" or something... so this would be trading inconvenience in one place (here) for multiple inconveniences down the road. basically I went with Roma's suggestion here. The question of idiomatic golang's version of "subclass & override one method" is still in the air for me...

Copy link
Contributor

@klizhentas klizhentas left a comment

Choose a reason for hiding this comment

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

looks good, although I would replace the struct with function properties with interfaces

@kontsevoy kontsevoy merged commit 7540f45 into master Sep 4, 2017
@kontsevoy kontsevoy deleted the ev/tctl branch September 4, 2017 19:39
hatched pushed a commit that referenced this pull request Jan 30, 2023
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