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

pkcs15-init,pkcs15-tool: reword --no-prompt to --use-pinpad (close #944) #957

Merged
merged 2 commits into from
Feb 3, 2017

Conversation

nunojpg
Copy link
Contributor

@nunojpg nunojpg commented Jan 31, 2017

Wording was confusing for a novice user. Old option is mantained as an alias,
but will print to stderr a deprecation warning.

Deprecation related code is all marked with deprecated word to easy future removal.

Signed-off-by: Nuno Goncalves [email protected]

@nunojpg nunojpg force-pushed the pkcs15-tool-use-pinpad-rename branch from 1339af1 to b18799e Compare January 31, 2017 17:02
Copy link
Member

@frankmorgner frankmorgner left a comment

Choose a reason for hiding this comment

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

The documentation in doc/tools needs to be updated.

@@ -201,7 +202,8 @@ const struct option options[] = {
{ "insecure", no_argument, NULL, OPT_INSECURE },
{ "use-default-transport-keys",
no_argument, NULL, 'T' },
{ "no-prompt", no_argument, NULL, OPT_NO_PROMPT },
{ "use-pinpad", no_argument, NULL, OPT_USE_PINPAD },
{ "no-prompt", no_argument, NULL, OPT_USE_PINPAD_DEPRECATED },
Copy link
Member

Choose a reason for hiding this comment

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

Please make it a silent option or simply remove the old option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, also thought it was the better. Just didn't find any example of a silent option, in this code. Can you point me how to do it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

owh, I found, if the help is string (const char*) NULL it becomes silent. Will fix it. Thanks.

@nunojpg
Copy link
Contributor Author

nunojpg commented Jan 31, 2017

AFAIK this option was not documented at all under doc/tools (I checked it.)

@nunojpg nunojpg force-pushed the pkcs15-tool-use-pinpad-rename branch from b18799e to 98c73a8 Compare February 1, 2017 09:44
@nunojpg
Copy link
Contributor Author

nunojpg commented Feb 1, 2017

@frankmorgner updated as advised.

Copy link
Member

@frankmorgner frankmorgner left a comment

Choose a reason for hiding this comment

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

Please also add documentation to doc/tools even if --no-prompt has not been there before. For a quick start, simply copy the help string to the manpage.

case OPT_NO_PROMPT:
opt_no_prompt = 1;
case OPT_USE_PINPAD_DEPRECATED:
fprintf(stderr, "--no-prompt have been deprecated and will be removed. Use instead '--use-pinpad'.\n");
Copy link
Member

Choose a reason for hiding this comment

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

Please change the messages to "'--no-prompt' is deprecated , use '--use-pinpad' instead.\n

@nunojpg nunojpg force-pushed the pkcs15-tool-use-pinpad-rename branch from 98c73a8 to beda702 Compare February 2, 2017 10:05
…enSC#944)

Wording was confusing for a novice user. Old option is mantained as an alias,
but will print to stderr a deprecation warning.

Deprecation related code is all marked with deprecated word to easy future removal.

Signed-off-by: Nuno Goncalves <[email protected]>
@nunojpg nunojpg force-pushed the pkcs15-tool-use-pinpad-rename branch from beda702 to c4b7a7e Compare February 2, 2017 10:08
@nunojpg
Copy link
Contributor Author

nunojpg commented Feb 2, 2017

@frankmorgner updated.

@frankmorgner
Copy link
Member

good work, thanks!

@frankmorgner frankmorgner merged commit 68f8f0b into OpenSC:master Feb 3, 2017
@nunojpg nunojpg deleted the pkcs15-tool-use-pinpad-rename branch February 3, 2017 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants