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.

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

Fix typo #13499

Merged
merged 1 commit into from
Mar 14, 2022
Merged

Fix typo #13499

merged 1 commit into from
Mar 14, 2022

Conversation

rst0git
Copy link
Contributor

@rst0git rst0git commented Mar 13, 2022

No description provided.

@zhangguanzhang
Copy link
Collaborator

code LGTM, bug title or commit messages need a [NO TEST NEEDED]

Copy link
Collaborator

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

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

LGTM but [NO NEW TESTS NEEDED] is needed as requested by @zhangguanzhang

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 14, 2022
[NO NEW TESTS NEEDED]

Signed-off-by: Radostin Stoyanov <[email protected]>
@rst0git
Copy link
Contributor Author

rst0git commented Mar 14, 2022

Thanks, I added [NO NEW TESTS NEEDED] in the commit message.

@rhatdan
Copy link
Member

rhatdan commented Mar 14, 2022

/approve
/lgtm
/hold
Thanks @rst0git

@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Mar 14, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, rst0git, zhangguanzhang

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:
  • OWNERS [rhatdan,zhangguanzhang]

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

@umohnani8
Copy link
Member

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 14, 2022
@openshift-merge-robot openshift-merge-robot merged commit 33d8081 into containers:main Mar 14, 2022
@Luap99
Copy link
Member

Luap99 commented Mar 14, 2022

This should be reverted in IMO, this is a breaking change for everyone using the binding with this parameter.
So if we want to keep the stability promise we cannot change this until 5.0!
@containers/podman-maintainers PTAL

@rst0git
Copy link
Contributor Author

rst0git commented Mar 14, 2022

this is a breaking change for everyone using the binding with this parameter.

@Luap99 As far as I know, podman-remote does not support container restore with --import.

cc: @adrianreber

@Luap99
Copy link
Member

Luap99 commented Mar 14, 2022

Well at least according to the code it is implemented and this is not about podman-remote.
pkg/bindings is used by other projects as dependency. We guarantee that this is stable and cannot break their code without a major release.
We discussed this as team and decided that we will add the old field back and alias it to the new correct name.

@vrothberg
Copy link
Member

@rst0git no wrong doing on your end. We should have caught this during review.

As @Luap99 mentions above, there are certain APIs that we committed to keep stable with pkg/bindings being among them. We'll keep this change but add back the previous field and API. There may be some callers of it that would otherwise break when updating to the next version.

@rhatdan
Copy link
Member

rhatdan commented Mar 15, 2022

We could keep the old name with a big comment and call into the new name.

@Luap99
Copy link
Member

Luap99 commented Mar 15, 2022

We could keep the old name with a big comment and call into the new name.

That is the plan.

@Luap99
Copy link
Member

Luap99 commented Mar 15, 2022

#13514

@rst0git rst0git deleted the fix-typo branch January 11, 2023 15:21
@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 15, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Development

Successfully merging this pull request may close these issues.

8 participants