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(nvmf): deprecate old nvmf cmdline options #1832

Merged
merged 2 commits into from
Jun 2, 2022

Conversation

aafeijoo-suse
Copy link
Member

  • Deprecate old nvmf cmdline options without the rd. prefix.
  • Add missing rd.nonvmf cmdline option to man page.

Checklist

  • I have tested it locally
  • I have reviewed and updated any documentation if relevant
  • I am providing new code and test(s) for it

Fixes #1831

@github-actions github-actions bot added modules Issue tracker for all modules nvmf Issues related to the nvmf module labels Jun 2, 2022
@johannbg
Copy link
Collaborator

johannbg commented Jun 2, 2022

@aafeijoo-suse hmm dont we need to provide a transition period from "old" to "new" encase there are setups in the wild using the non rd prefixed parameters?

@johannbg
Copy link
Collaborator

johannbg commented Jun 2, 2022

@aafeijoo-suse dont get me wrong I'm not saying we should since I'm perfectly aware that people wont adapt until they have to so I personally would want to rip the bandage off so to speak immediately instead of allowing "transition" period of some sort so I'm fine with merging this either way.

Copy link
Collaborator

@johannbg johannbg left a comment

Choose a reason for hiding this comment

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

LGTM

@aafeijoo-suse
Copy link
Member Author

You made me hesitate so I had to double-check it, but adding -d with the old option name to getarg just prints a deprecation warning and this old option name doesn't stop working.

@johannbg
Copy link
Collaborator

johannbg commented Jun 2, 2022

@aafeijoo-suse yeah it worked just fine, I'm just conflicted if we should or should not be providing backwards compatibilty with the old prefixes thus I needed a second opinion on the matter. ( sorry for getting you confused, still waking up on this end ;) )

@johannbg johannbg merged commit 7742052 into dracutdevs:master Jun 2, 2022
@aafeijoo-suse
Copy link
Member Author

@aafeijoo-suse hmm dont we need to provide a transition period from "old" to "new" encase there are setups in the wild using the non rd prefixed parameters?

Oh, the "don't" instead of "do" made me misunderstand your question :)

Yes, we know that allowing deprecated options without announcing an expiration date or something keeps people using it, but in this case would be too abrupt in my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules Issue tracker for all modules nvmf Issues related to the nvmf module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Man page quotes non-functional nvmf parameter names
2 participants