Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Improve SDK documentation #415
Improve SDK documentation #415
Changes from 8 commits
d8a3685
d368406
249515e
a868455
a3b4385
506bc5a
11b7e87
fb575ce
ff63e6b
d254815
6cf04be
8b5c81c
09a4f28
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
thought: I know this is an established format but to me
ClientGenerator
sounds weird, like it's something that generates clients. It's the same thing with the other client extensions, e.g.ClientVault
which sounds like there are multiple vaults and this one belongs to theClient
.On the other hand
ClientGeneratorExt
sounds very reasonable, it's aGenerator Extension
for theClient
.I don't think I would've reacted to this if we would've just passed around the
Client
struct and simply appliedExtensions
to it, i.e.Client.password(...)
, but as it is now, we are actually creating secondary "sub-clientswhich themself have a reference to the
Clientinstance.
Client.generator()` actively constructs a new type of "Client"Maybe I'm just nitpicking I don't know, but I think we'll have problems referring to things in the future
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'm open for alternative name schemas.
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.
@Hinton
Well my suggestion is to flip the name from ClientGenerator to GeneratorClient, not that big change considering how big my previous message was xD
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.
Seems fine to me. Mind creating a jira issue and tackling this mountain of a task :).
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.
@Hinton Done! PM-13166 Rename SDK sub-clients to
<>Client
instead ofClient<>