Skip to content

Remove gogoproto.stdtime#6821

Merged
estesp merged 1 commit intocontainerd:mainfrom
kzys:remove-std-time
Apr 19, 2022
Merged

Remove gogoproto.stdtime#6821
estesp merged 1 commit intocontainerd:mainfrom
kzys:remove-std-time

Conversation

@kzys
Copy link
Copy Markdown
Member

@kzys kzys commented Apr 18, 2022

This commit removes gogoproto.stdtime, since it is not supported by
Google's official toolchain
(see #6564).

Signed-off-by: Kazuyoshi Kato [email protected]

@kzys kzys force-pushed the remove-std-time branch 2 times, most recently from f74304e to 96226d9 Compare April 18, 2022 22:32
Comment thread protobuf/timestamp.go
}

// FromTimestamp creates time.Time from protobuf's Timestamp
func FromTimestamp(from *types.Timestamp) time.Time {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note that protobuf/types already implements this: https://github.com/gogo/protobuf/blob/b03c65ea87cdc3521ede29f62fe3ce239267c1bc/types/timestamp.go#L88-L98

There's some validation there (which I didn't dig into yet), but the other case is a nil timestamp.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh, how didn't I notice that. Let me fix.

Comment thread protobuf/timestamp.go Outdated
// ToTimestamp creates protobuf's Timestamp from time.Time
func ToTimestamp(from time.Time) *types.Timestamp {
return &types.Timestamp{
Seconds: int64(from.Second()),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note that protobuf/types does this as Seconds: from.Unix()

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 18, 2022

Build succeeded.

@kzys kzys force-pushed the remove-std-time branch from 96226d9 to f542c99 Compare April 18, 2022 23:09
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 18, 2022

Build succeeded.

@kzys kzys force-pushed the remove-std-time branch from f542c99 to f17e409 Compare April 18, 2022 23:37
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 18, 2022

Build succeeded.

@kzys kzys force-pushed the remove-std-time branch from f17e409 to 3a2ab52 Compare April 18, 2022 23:56
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 19, 2022

Build succeeded.

@kzys kzys force-pushed the remove-std-time branch from 3a2ab52 to 8ff0110 Compare April 19, 2022 02:00
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 19, 2022

Build succeeded.

@thaJeztah
Copy link
Copy Markdown
Member

Looks like this needs a rebase, @kzys

@kzys kzys closed this Apr 19, 2022
@kzys kzys force-pushed the remove-std-time branch from 8ff0110 to 26a3ab4 Compare April 19, 2022 13:29
@kzys kzys reopened this Apr 19, 2022
This commit removes gogoproto.stdtime, since it is not supported by
Google's official toolchain
(see containerd#6564).

Signed-off-by: Kazuyoshi Kato <[email protected]>
@kzys kzys force-pushed the remove-std-time branch from f229e84 to 80b825c Compare April 19, 2022 13:39
@kzys
Copy link
Copy Markdown
Member Author

kzys commented Apr 19, 2022

@thaJeztah Thanks. Merged sandbox changes and removed stdtime from the new protos as well.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 19, 2022

Build succeeded.

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 184883b into containerd:main Apr 19, 2022
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.

5 participants