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

network create: add warning for deprecated macvlan flag #11402

Merged
merged 1 commit into from
Sep 7, 2021

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Sep 2, 2021

The macvlan driver is not deprecated, only the --macvlan flag is.
Remove the flag from the man page since it is deprecated and add a
warning to podman network create if it is used.

Fixes #11400

Signed-off-by: Paul Holzinger [email protected]

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 2, 2021
@@ -54,9 +55,9 @@ must be used with a *subnet* option.

Set metadata for a network (e.g., --label mykey=value).

#### **--macvlan**
#### **--macvlan**=*device*
Copy link
Member

Choose a reason for hiding this comment

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

I think we should just remove the documentation rather then telling users about it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would like to keep it as long as the option still works. I want to remove this option for 4.0.

Copy link
Member

Choose a reason for hiding this comment

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

The option can still be there, just no reason to document it. That way existing users will continue to use it, but no new users will accidentally start using it.

Copy link
Member

Choose a reason for hiding this comment

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

Or perhaps we can add a "deprecated" note to this and then say as of 4.0 it's being removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think deprecated options should still be documented. This change clearly states this and what you should use instead. I could add that it is being removed with podman 4.0 but I am not sure if there is any reason to remove it.

Copy link
Member

Choose a reason for hiding this comment

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

I disagree, if we don't want people to use or rely on an option, we should make the option disappear. 4.0 would allow us to break their scripts(Although I would recommend against that).

We can document in the release notes that it is depracated, and print a warning when users use it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the flag from the man page and emit a warning when --macvlan is used.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 2, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, 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

@mheon
Copy link
Member

mheon commented Sep 3, 2021

LGTM

@Luap99 Luap99 changed the title [CI:DOCS] Fix podman network create doc for macvlan Fix podman network create doc for macvlan Sep 6, 2021
@Luap99 Luap99 force-pushed the macvlan-doc branch 3 times, most recently from 1ecd262 to cd8c86e Compare September 7, 2021 08:23
@Luap99 Luap99 changed the title Fix podman network create doc for macvlan network create: add warning for deprecated macvlan flag Sep 7, 2021
The macvlan driver is not deprecated, only the --macvlan flag is.
Remove the flag from the man page since it is deprecated and add a
warning to podman network create if it is used.

[NO TESTS NEEDED]

Fixes containers#11400

Signed-off-by: Paul Holzinger <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Sep 7, 2021

Thanks @Luap99
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 7, 2021
@openshift-merge-robot openshift-merge-robot merged commit 6e3a2d3 into containers:main Sep 7, 2021
@Luap99 Luap99 deleted the macvlan-doc branch September 7, 2021 11:14
@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 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 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.

Documentation indicates that the --driver option only supports bridged networking.
5 participants