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

Tweak OpenAPI spec embedding (again) #67

Merged
merged 1 commit into from Aug 1, 2019
Merged

Conversation

ob-stripe
Copy link
Contributor

Reviewers

r? @tomer-stripe @brandur-stripe
cc @stripe/dev-platform

Summary

Sorry for all the back and forth... Hopefully this will work.

This is basically a revert of #58, except that:

  • the build tag for the Go generation script is now vfsgen instead of ignore. ignore is a special value which causes the package to be completely ignored and thus the dependencies were omitted from go.mod. By setting it to a different value (vfsgen, to be consistent with the file name, but it could really be anything), the dependencies are properly included.
  • we override the modification times to be the Unix epoch (using sample code provided by vfsgen's author here). git does not manage mod times, so the mod times can change between different environments, which results in go generate ./... producing different results. This ensures that the generated fs_vfsdata.go file will only change when the file contents are actually updated.

@brandur-stripe
Copy link

we override the modification times to be the Unix epoch (using sample code provided by vfsgen's author here). git does not manage mod times, so the mod times can change between different environments, which results in go generate ./... producing different results. This ensures that the generated fs_vfsdata.go file will only change when the file contents are actually updated.

Would you mind putting this explanation in the file somewhere? Just reading through that code, it wasn't obvious to me at all why we'd bother with all this mod time stuff, and I think it's going to be easy to forget why this is done a few months from now.

Nice fix though! LGTM otherwise.

@ob-stripe
Copy link
Contributor Author

@ob-stripe ob-stripe merged commit 5e8ad17 into master Aug 1, 2019
@ob-stripe ob-stripe deleted the ob-vfsgen-ignore-modtimes branch August 1, 2019 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants