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

Switch all references to image.ContainerConfig to image.Config #2023

Merged
merged 1 commit into from
Dec 22, 2018
Merged

Switch all references to image.ContainerConfig to image.Config #2023

merged 1 commit into from
Dec 22, 2018

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Dec 18, 2018

This will more closely match what Docker is doing.

Signed-off-by: Daniel J Walsh dwalsh@redhat.com

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: rhatdan

If they are not already assigned, you can assign the PR to them by writing /assign @rhatdan in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rhatdan
Copy link
Member Author

rhatdan commented Dec 18, 2018

Fixes: #2017

@rhatdan
Copy link
Member Author

rhatdan commented Dec 18, 2018

@baude PTAL

@rhatdan rhatdan force-pushed the config branch 3 times, most recently from 4f7dc3e to 184fd8d Compare December 18, 2018 17:22
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L labels Dec 18, 2018
@mheon
Copy link
Member

mheon commented Dec 18, 2018

Whole lot of vendor changes in here - intentional?

@rhatdan
Copy link
Member Author

rhatdan commented Dec 18, 2018

Nope will fix.

@openshift-ci-robot openshift-ci-robot added size/L and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 18, 2018
@rhatdan
Copy link
Member Author

rhatdan commented Dec 18, 2018

@cevich The version in the cirrus.yml file seems to be old. We need golang 1.11 to make validate work correctly.

@baude
Copy link
Member

baude commented Dec 18, 2018

@rhatdan this is not for @cevich , it's for me and the origin team. At last check they didn't have a 1.11 image we could test against. I'll engage again today.

@baude
Copy link
Member

baude commented Dec 18, 2018

@rhatdan revert the cmd/podman/rmi.go file and it will pass ... nothing is going to change quickly here.

@rhatdan rhatdan force-pushed the config branch 2 times, most recently from 6ecaedf to 788bf19 Compare December 19, 2018 13:51
@rhatdan
Copy link
Member Author

rhatdan commented Dec 19, 2018

/retest

@baude
Copy link
Member

baude commented Dec 20, 2018

rebase and fix tests and should be good to go

@rhatdan rhatdan force-pushed the config branch 3 times, most recently from 196ac90 to 5cb65e5 Compare December 21, 2018 15:17
@rhatdan
Switch all referencs to image.ContainerConfig to image.Config
c657dc4 
This will more closely match what Docker is doing.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: rhatdan

If they are not already assigned, you can assign the PR to them by writing /assign @rhatdan in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rhatdan
Copy link
Member Author

rhatdan commented Dec 21, 2018

/approve

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 21, 2018
@rhatdan
Copy link
Member Author

rhatdan commented Dec 21, 2018

@baude @mheon @umohnani8 @giuseppe @vrothberg @TomSweeneyRedHat PTAL
All Green.

@mheon
Copy link
Member

mheon commented Dec 21, 2018

Sure, LGTM. One more and we can merge.

@TomSweeneyRedHat TomSweeneyRedHat changed the title Switch all referencs to image.ContainerConfig to image.Config Switch all references to image.ContainerConfig to image.Config Dec 22, 2018
TomSweeneyRedHat
TomSweeneyRedHat reviewed Dec 22, 2018
View reviewed changes
Copy link
Member

@TomSweeneyRedHat TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

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

LGTM
I actually preferred the "ContanerConfig" in the output, but think I'm probably in the minority and I know this lines up better conformance wise.

@TomSweeneyRedHat
Copy link
Member

And all happy green test buttons.

@vrothberg
Copy link
Member

/lgtm

@mheon, we should mention this in the release notes in case some users relied on the ContainerConfig key.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 22, 2018
@openshift-merge-robot openshift-merge-robot merged commit 792f109 into containers:master Dec 22, 2018
@mheon
Copy link
Member

mheon commented Dec 22, 2018

@vrothberg Ack, I'll add a compatability note for it

pkubatrh added a commit to pkubatrh/container-common-scripts that referenced this pull request Jan 23, 2019
@pkubatrh
replace .ContainerConfig by .Config
f4b2cc1 
This is due to a change in podman 1.0:
containers/podman#2023
@pkubatrh pkubatrh mentioned this pull request Jan 23, 2019
Merged
pkubatrh added a commit to sclorg/container-common-scripts that referenced this pull request Jan 29, 2019
@pkubatrh
replace .ContainerConfig by .Config
a7759b9 
This is due to a change in podman 1.0:
containers/podman#2023
pkubatrh added a commit to pkubatrh/s2i-ruby-container that referenced this pull request Mar 5, 2019
@pkubatrh
test: use Config instead of ContainerConfig
bc11981 
This is due to a change in podman 1.0:
containers/podman#2023
@pkubatrh pkubatrh mentioned this pull request Mar 5, 2019
Merged
pkubatrh added a commit to sclorg/s2i-ruby-container that referenced this pull request Mar 6, 2019
@pkubatrh
test: use Config instead of ContainerConfig
cab038c 
This is due to a change in podman 1.0:
containers/podman#2023
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 27, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@TomSweeneyRedHat TomSweeneyRedHat TomSweeneyRedHat left review comments

@umohnani8 umohnani8 Awaiting requested review from umohnani8

Assignees

@vrothberg vrothberg

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@rhatdan @openshift-ci-robot @mheon @baude @TomSweeneyRedHat @vrothberg @openshift-merge-robot