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

list-modules: Do not embed module-name and module-path in URL #557

Merged
merged 2 commits into from
Sep 18, 2023

Conversation

ueno
Copy link
Member

@ueno ueno commented Sep 18, 2023

module-name and module-path are query attributes, which are to modify
the default settings (RFC 7512 section 2.4). As p11-kit manages the
default configuration, it shouldn't print them as part of the token
URL. This instead prints the absolute path of each module in a
separate field of the output.

As we no longer call `set -e`, exit status of each command invocation
should be checked manually.

Signed-off-by: Daiki Ueno <[email protected]>
Copy link
Contributor

@ZoltanFridrich ZoltanFridrich left a comment

Choose a reason for hiding this comment

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

The change looks good.

module-name and module-path are query attributes, which are to modify
the default settings (RFC 7512 section 2.4).  As p11-kit manages the
default configuration, it shouldn't print them as part of the token
URL.  This instead prints the absolute path of each module in a
separate field of the output.

Signed-off-by: Daiki Ueno <[email protected]>
@ueno ueno force-pushed the wip/dueno/list-modules branch from 89c2115 to 14763e4 Compare September 18, 2023 09:24
@coveralls
Copy link

Coverage Status

coverage: 68.917% (+0.3%) from 68.648% when pulling 14763e4 on ueno:wip/dueno/list-modules into ca5ed35 on p11-glue:master.

@ueno ueno merged commit 79fe36c into p11-glue:master Sep 18, 2023
12 checks passed
@ueno ueno added this to the 0.25.1 milestone Oct 19, 2023
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.

3 participants