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

Switch protobuf module to google.golang.org/protobuf #452

Merged
merged 2 commits into from
Dec 10, 2020

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Dec 9, 2020

The first commit is a preparatory cleanup removing pkg/printer.MaybeTime. Note that this is an exported func, but there are no known external users, so it should be safe to drop it.

The second commit switches from the deprecated github.com/golang/protobuf module to google.golang.org/protobuf.

@tklauser tklauser added 🧹 kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. labels Dec 9, 2020
t, err := ptypes.Timestamp(ts)
if err != nil || t.IsZero() {
func fmtTimestamp(ts *timestamppb.Timestamp) string {
if !ts.IsValid() {
Copy link
Member

Choose a reason for hiding this comment

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

Very nice find. For other reviewers: IsValid checks for nil and invalid timestamps both.

@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Dec 9, 2020
@tklauser tklauser added the :basecamp: needs-rebase This PR needs to be rebased because it has merge conflicts. label Dec 10, 2020
MaybeTime is only used in fmtTimestamp. The timer.Time passed to it
can never be nil so the "N/A" case is never hit (except in tests).
Thus, pull the time.Format call directly into fmtTimestamp.

Note that MaybeTime is an exported func, but there are no known external
users, so it should be safe to drop it.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Update all users of the deprecated github.com/golang/protobuf module to
google.golang.org/protobuf

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@tklauser
Copy link
Member Author

FYI, rebased on top of latest master to resolve conflicts due to merge of #442 and #452.

@tklauser tklauser removed the :basecamp: needs-rebase This PR needs to be rebased because it has merge conflicts. label Dec 10, 2020
@gandro gandro merged commit da93a5f into master Dec 10, 2020
@gandro gandro deleted the pr/tklauser/protobuf-apiv2 branch December 10, 2020 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧹 kind/cleanup This includes no functional changes. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants