Skip to content

Add automatic platform args in the global scope#142

Merged
openshift-merge-robot merged 2 commits intoopenshift:masterfrom
grooverdan:validate_expand_target_platform
Nov 15, 2019
Merged

Add automatic platform args in the global scope#142
openshift-merge-robot merged 2 commits intoopenshift:masterfrom
grooverdan:validate_expand_target_platform

Conversation

@grooverdan
Copy link
Copy Markdown
Contributor

@grooverdan grooverdan commented Oct 10, 2019

Based on https://docs.docker.com/engine/reference/builder/#automatic-platform-args-in-the-global-scope

By default assumed build platform = target platform as builder doesn't [yet?] gave platform.

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 10, 2019
@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Oct 10, 2019

Hi @grooverdan. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 10, 2019
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 10, 2019
@grooverdan
vendor github.com/containerd/containerd
91f3c94 
Signed-off-by: Daniel Black <daniel@linux.ibm.com>

fix vendor
@grooverdan
Add automatic platform args in the global scope
54d2f38 
Signed-off-by: Daniel Black <daniel@linux.ibm.com>
@TomSweeneyRedHat
Copy link
Copy Markdown
Contributor

TomSweeneyRedHat commented Oct 10, 2019

/ok-to-test

@openshift-ci-robot openshift-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 10, 2019
@TomSweeneyRedHat
Copy link
Copy Markdown
Contributor

TomSweeneyRedHat commented Oct 10, 2019

In Docker, this is only supported starting with v18.09 while using BuildKit. The changes here look reasonable to me, @nalind WDYT?

@TomSweeneyRedHat TomSweeneyRedHat mentioned this pull request Oct 10, 2019
Closed
@rhatdan
Copy link
Copy Markdown
Contributor

rhatdan commented Oct 10, 2019

@mrunalp WDYT?

grooverdan added a commit to grooverdan/buildah that referenced this pull request Oct 11, 2019
@grooverdan
openshift: imagebuilder args (TODO - drop patch)
9411717 
use openshift/imagebuilder#142

Signed-off-by: Daniel Black <daniel@linux.ibm.com>
@grooverdan
Copy link
Copy Markdown
Contributor Author

grooverdan commented Oct 25, 2019

Any objections @nalind, @mrunalp ?

@rhatdan
Copy link
Copy Markdown
Contributor

rhatdan commented Oct 29, 2019

/approve

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 29, 2019
@grooverdan
Copy link
Copy Markdown
Contributor Author

grooverdan commented Oct 31, 2019 •
edited
Loading

So this is missing some /lgtm and removing the /size/xxl ❤️ .

@grooverdan
Copy link
Copy Markdown
Contributor Author

grooverdan commented Oct 31, 2019

/joke

@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Oct 31, 2019

@grooverdan: Why did the man run around his bed? Because he was trying to catch up on his sleep!

Details

In response to this:

/joke

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@grooverdan
Copy link
Copy Markdown
Contributor Author

grooverdan commented Nov 6, 2019

🚥

/joke

@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Nov 6, 2019

@grooverdan: Why does Waldo only wear stripes? Because he doesn't want to be spotted.

Details

In response to this:

🚥

/joke

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@grooverdan
Copy link
Copy Markdown
Contributor Author

grooverdan commented Nov 12, 2019

Can you push the vendor github.com/containerd/containerd updates separately? Then I can remove that from the series. That will remove the size/XXL tag enabling this to be merged.

@TomSweeneyRedHat
Copy link
Copy Markdown
Contributor

TomSweeneyRedHat commented Nov 13, 2019

@nalind @mrunalp @RedHatDan PTAL

@rhatdan
Copy link
Copy Markdown
Contributor

rhatdan commented Nov 15, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 15, 2019
@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Nov 15, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: grooverdan, rhatdan

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

The pull request process is described here

Details 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-merge-robot openshift-merge-robot merged commit 2256314 into openshift:master Nov 15, 2019
@TomSweeneyRedHat
Copy link
Copy Markdown
Contributor

TomSweeneyRedHat commented Nov 15, 2019

And scrolling back I realize I never LGTM'd. So doing so now.
LGTM

@grooverdan grooverdan deleted the validate_expand_target_platform branch November 17, 2019 23:06
@nalind nalind mentioned this pull request Jan 31, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rhatdan rhatdan Awaiting requested review from rhatdan

@TomSweeneyRedHat TomSweeneyRedHat Awaiting requested review from TomSweeneyRedHat

Assignees

@rhatdan rhatdan

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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@grooverdan @openshift-ci-robot @TomSweeneyRedHat @rhatdan @openshift-merge-robot