-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Move to gogoproto #715
Move to gogoproto #715
Conversation
This switches the nflog to generate Go code via gogoproto and thereby use standard library timestamp types.
scripts/genproto.sh
Outdated
|
||
#!/usr/bin/env bash | ||
# | ||
# Generate all etcd protobuf bindings. |
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.
Why do we reference etcd
here?
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.
Stolen script ;)
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.
What's the license on this file?
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.
etcd script: https://github.com/coreos/etcd/blob/master/scripts/genproto.sh
Actually just took it as a skeleton to play around in. No license on this file.
What's the proper way to proceed here in your opinion?
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.
The source repo is all Apache 2.0, so I'd just add a one-line note of where it was originally taken from and under what license.
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.
Done.
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.
Barring the raised license concern, looks cool to me
This generates the protobuf Go code with gogoproto and switches to standard library time types.
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.
👍
scripts/genproto.sh
Outdated
# Generate all protobuf bindings. | ||
# Run from repository root. | ||
# | ||
# Initial script take from etcd under the Apache 2.0 license |
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.
taken
@stuartnelson3 @mxinden
This moves our generated protobuf to gogoproto, which allows us to get rid of the awkward time types. I'd like to get this in before a release of the notification fixes.