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

Refactor samples.go to be more configurable #1016

Merged
merged 3 commits into from
Jan 9, 2023

Conversation

richardm-stripe
Copy link
Collaborator

@richardm-stripe richardm-stripe commented Jan 6, 2023

Reviewers

r? @vcheung-stripe
cc @stripe/developer-products

Summary

Refactors the samples command so that the code can be re-used/configured via plugin.

Specifically

  • I renamed the Samples struct to SampleManager as identifier samples was referring both to the package and to an instance of Samples which I found confusing.
  • I gave SampleManager a constructor helper, NewSampleManager with a default initialization, rather than having it initialized directly everywhere.
  • I changed the Create and GetSampleConfig functions to be methods with a SampleManager receiver, so that they are more configurable.
  • I refactored ConfigureDotEnv into SampleManager.ConfigureDotEnv (which can be given a custom implementation, now that it's on the struct), and WriteDotEnv.
  • I introduced a SampleLister interface to replace getSamples, so that this can be customized (e.g., an implementation that returns a hardcoded list instead of fetching and caching from a specific github repository) and refactored the existing implementation into cachedGithubSampleLister.

Testing

As a basic sanity check, I manually ran stripe samples list and stripe samples create and they appear still to work.

@richardm-stripe richardm-stripe requested a review from a team as a code owner January 6, 2023 20:45
Copy link
Contributor

@gracegoo-stripe gracegoo-stripe left a comment

Choose a reason for hiding this comment

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

Thanks for refactoring this, LGTM!

@richardm-stripe richardm-stripe merged commit 71b7e38 into master Jan 9, 2023
@richardm-stripe richardm-stripe deleted the richardm-samples-configurable branch January 9, 2023 16:45
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.

None yet

2 participants