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.

Sign up for GitHub

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

Jump to bottom

Refactor v1beta3 - Refactor 'Metadata' to ObjectMeta from all types #2102

Merged
merged 1 commit into from
Nov 3, 2014
Merged

Refactor v1beta3 - Refactor 'Metadata' to ObjectMeta from all types #2102

merged 1 commit into from
Nov 3, 2014

Conversation

markturansky
Copy link
Contributor

Removed "Metadata" from all domain objects, leaving just "ObjectMeta" with each.

@markturansky markturansky changed the title Removed 'Metadata' from all types Refactor v1beta3 - Removd 'Metadata' from all types Oct 31, 2014
@markturansky markturansky changed the title Refactor v1beta3 - Removd 'Metadata' from all types Refactor v1beta3 - Remove 'Metadata' from all types Oct 31, 2014
@lavalamp
Copy link
Member

Is there some history on why we're doing this?

bgrant0607
bgrant0607 reviewed Oct 31, 2014
View reviewed changes
pkg/api/v1beta3/types.go
@@ -473,7 +473,7 @@ type PodStatus struct {
// to hosts.
type Pod struct {
TypeMeta `json:",inline" yaml:",inline"`
Metadata ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty"`
ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Don't all these need to say "inline" rather than "omitempty"?

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind. @smarterclayton is right. This should stay omitempty, so that the serialization doesn't change.

@markturansky markturansky force-pushed the v1beta3_refactor_metadata branch from 1fce6c5 to 90c436f Compare October 31, 2014 20:47
@smarterclayton
Copy link
Contributor

For reference, discussion on IRC about benefits of leaving JSON serialization as "metadata" vs code benefits internally of embedding the type https://botbot.me/freenode/google-containers/msg/24613191/

@markturansky markturansky force-pushed the v1beta3_refactor_metadata branch 2 times, most recently from 9fd2641 to 9140d83 Compare October 31, 2014 20:56
@bgrant0607
Copy link
Member

LGTM

lavalamp
lavalamp reviewed Oct 31, 2014
View reviewed changes
pkg/api/v1beta3/types.go
@@ -486,7 +486,7 @@ type Pod struct {
// PodList is a list of Pods.
type PodList struct {
TypeMeta `json:",inline" yaml:",inline"`
Metadata ListMeta `json:"metadata" yaml:"metadata"`
ListMeta `json:"metadata" yaml:"metadata"`
Copy link
Member

Choose a reason for hiding this comment

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

Why does listmeta not get omitempty?

Copy link
Member

Choose a reason for hiding this comment

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

No good reason, AFAIK. Didn't have it before. Probably oversight during the big v1beta3 PR.

@bgrant0607 bgrant0607 self-assigned this Oct 31, 2014
@markturansky markturansky force-pushed the v1beta3_refactor_metadata branch from 9140d83 to b44add9 Compare November 1, 2014 12:53
@markturansky
Copy link
Contributor Author

I added "omitempty" to each ListMeta that was missing it.

@smarterclayton
Copy link
Contributor

LGTM

@smarterclayton
Copy link
Contributor

Cleanup from earlier pulls for v1beta3 consistent with #1975. Partially extracted from #1600.

@thockin
Copy link
Member

thockin commented Nov 3, 2014

LGTM, though I fall in the "prefer to inline it" camp, I'll cope.

@markturansky markturansky force-pushed the v1beta3_refactor_metadata branch from b44add9 to 2260b24 Compare November 3, 2014 13:48
@markturansky markturansky changed the title Refactor v1beta3 - Remove 'Metadata' from all types Refactor v1beta3 - Refactor 'Metadata' to ObjectMeta from all types Nov 3, 2014
@smarterclayton
Copy link
Contributor

Merging before we reopen all other design discussions :)

smarterclayton added a commit that referenced this pull request Nov 3, 2014
@smarterclayton
Merge pull request #2102 from markturansky/v1beta3_refactor_metadata
d9c0b45 
Refactor v1beta3 - Refactor 'Metadata' to ObjectMeta from all types
@smarterclayton smarterclayton merged commit d9c0b45 into kubernetes:master Nov 3, 2014
@bgrant0607 bgrant0607 mentioned this pull request Nov 3, 2014
Merged
Tal-or pushed a commit to Tal-or/kubernetes that referenced this pull request Oct 9, 2024
@openshift-merge-bot
Merge pull request kubernetes#2102 from bertinatto/update-hyperkube-d…
8540bd4 
…ockerfile

WRKLDS-1449: Update Hyperkube Dockerfile with the right Kubernetes version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@bgrant0607 bgrant0607

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@markturansky @lavalamp @smarterclayton @bgrant0607 @thockin