Skip to content

Conversation

@wking
Copy link
Contributor

@wking wking commented Jun 27, 2018

Catching up with opencontainers/runtime-tools@84a62c6a (opencontainers/runtime-tools#266), now that we've bumped runtime-tools in f6c0fc1 (#1007).

CC @baude.

@wking wking mentioned this pull request Jun 27, 2018
Copy link
Member

Choose a reason for hiding this comment

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

This is a semantic change from before - we were using the annotations from the config supplied when creating the container libpod, not the annotations in the final/completed spec used to initialize the OCI runtime. I don't necessarily think this is a bad change - it's counterintuitive for people to see an environment variable in the container at runtime but not be able to trigger hooks off it - but work noting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a semantic change from before...

Oops, I only meant to adjust the first argument. Fixed with 4f49ffd5 -> 4e7d15f.

I don't necessarily think this is a bad change...

I don't mind if you want to file a follow-up PR to make this semantic pivot; but I don't think it should be mixed up with this PR's refactor.

Catching up with opencontainers/runtime-tools@84a62c6a (generate: Move
Generator.spec to Generator.Config, 2016-11-06, containers#266, v0.6.0), now
that we've bumped runtime-tools in f6c0fc1 (Vendor in latest
runtime-tools, 2018-06-26, containers#1007).

Signed-off-by: W. Trevor King <[email protected]>
@wking wking force-pushed the runtime-tools-spec-deprecated branch from 4f49ffd to 4e7d15f Compare June 27, 2018 17:11
@rhatdan
Copy link
Member

rhatdan commented Jun 27, 2018

LGTM

@mheon
Copy link
Member

mheon commented Jun 27, 2018

LGTM
@rh-atomic-bot r+

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 4e7d15f has been approved by mheon

@rh-atomic-bot
Copy link
Collaborator

⚡ Test exempted: pull fully rebased and already tested.

@wking wking deleted the runtime-tools-spec-deprecated branch June 28, 2018 04:29
@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.

Labels

locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants