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

Specify PathBuf type for blockdevs in parser #3642

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

mulkieran
Copy link
Member

No description provided.

@mulkieran mulkieran self-assigned this Jul 5, 2024
@mulkieran mulkieran marked this pull request as ready for review July 22, 2024 18:22
@mulkieran mulkieran requested a review from jbaublitz July 22, 2024 18:25
Copy link

Cockpit tests failed for commit 72829ae. @martinpitt, @jelly, @mvollmer please check.

@mulkieran
Copy link
Member Author

Failures are due to Cockpit test error, looks like.

@martinpitt
Copy link
Contributor

F40 passed, and the rawhide error is totally unrelated to stratis/cockpit:

+ systemctl start firewalld
+ firewall-cmd --add-service=cockpit --permanent
Error: [Errno 13] Permission denied: '/etc/firewalld/zones/public.xml'

That's almost surely some SELinux related regression. Please ignore. I'll try to reproduce this locally and report it.

@martinpitt
Copy link
Contributor

See https://bodhi.fedoraproject.org/updates/FEDORA-2024-aa10dc94fe . The automated tests are a sea of red.

Note that this does virtually nothing. A PathBuf is basically just an
OsString with extra behaviors. And it can always be parsed from a
String, which is the clap default argument value type.

All this change accomplishes is that when processing the argument values,
the paths will not have to be converted to Strings and then Pathbufs
because that conversion happened when they were read, because clap was
instructed to do it.

This change just lays the foundation for good practice where it might be
noticeable, so that when clap makes it possible, other values, e.g., UUIDs,
can be checked for proper format by the parser, and if the format is wrong,
a parser error will be returned as part of the parser operation.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran
Copy link
Member Author

rebased

@mulkieran mulkieran merged commit 96d94c4 into stratis-storage:master Jul 23, 2024
46 checks passed
@mulkieran mulkieran deleted the specify-types-in-parser branch July 23, 2024 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants