Skip to content
This repository was archived by the owner on May 30, 2024. It is now read-only.

Conversation

@tstone
Copy link

@tstone tstone commented Jan 31, 2020

Requirements

  • N/A I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • N/A I have validated my changes against all supported platform versions

I'm marking the tasks here N/A as I only added documentation comments. This PR includes no behavior changes.

Describe the solution you've provided

Typescript engineers who rely primarily on the exported type definitions might miss that the LDClient should only be initialized as a singleton. This PR clarifies that in the inline documentation.

Describe alternatives you've considered

Provide a clear and concise description of any alternative solutions or features you've considered.

Additional context

Missing this detail, a recent implementation initialized the LDClient on each request.

@eli-darkly
Copy link
Contributor

We're going to add a sentence about this, but using more concise text that follows our style and is consistent with the other SDKs. This topic is already covered in the main Getting Started docs.

@eli-darkly eli-darkly closed this Feb 11, 2020
@eli-darkly
Copy link
Contributor

To expand on that a little: in general our approach to the SDK API docs has been to document the semantics of each SDK type and method, rather than overall usage issue that are common to the LaunchDarkly platform as a whole. We strongly recommend reading the online docs (which are linked from the README file), rather than just picking out one of the SDK repositories on GitHub and diving in using only the in-code docs.

(Also, in this particular case the documentation for init already says "The client will begin attempting to connect to LaunchDarkly as soon as it is created." It would not be feasible to do that for every single request.)

LaunchDarklyCI pushed a commit that referenced this pull request Feb 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants