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

Merge changes to move server into its own package #265

Merged
merged 15 commits into from
Aug 10, 2021

Conversation

akalinin-stripe
Copy link
Contributor

@akalinin-stripe akalinin-stripe commented Aug 10, 2021

Notify

r? @richardm-stripe
cc @dcr-stripe @stripe/api-libraries

Summary

Based on the changes from #238 (thank you @Splizard) with conflicts resolved against the master branch.

The original intent was to fix issue #206 and allow running embedded version of the server.
Detailed changes:

  • Move server code from main package into /server directory
  • Split embedded data generation to embed spec in server directory and certificates only in main directory
  • Refactor out global state verbose variable and include it into server, so it does not become part of the API (not sure what to do with Version though, usually it is OK to leave it as it is set by ldflags).

Finally, it looks like it's a lot of changes, but mostly it's shuffling code around to a different package.

Testing

  • Unit tests updated
  • Ran tests with the current stripe-go repo

@akalinin-stripe
Copy link
Contributor Author

False positive in golint is catching auto-generated file comment:
sh go list ./... | xargs -I{} -n1 sh -c 'golint -set_exit_status {} || exit 255' /home/travis/gopath/src/github.com/stripe/stripe-mock/server/bindata.go:1:1: package comment should be of the form "Package server ..."

Makefile Outdated Show resolved Hide resolved
Copy link
Contributor

@richardm-stripe richardm-stripe left a comment

Choose a reason for hiding this comment

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

This all looks good! The only thing is that I think we should remove bindata.go as the path has moved.

main.go Show resolved Hide resolved
server/server.go Outdated Show resolved Hide resolved
server/server.go Outdated Show resolved Hide resolved
Copy link
Contributor

@richardm-stripe richardm-stripe left a comment

Choose a reason for hiding this comment

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

Clarified IRL! I understand the bindata commands now! Good to merge!

akalinin-stripe and others added 2 commits August 10, 2021 13:30
Co-authored-by: Richard Marmorstein <[email protected]>
Co-authored-by: Richard Marmorstein <[email protected]>
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.

3 participants