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

Replace sudo param with maybe_sudo method. #22

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Replace sudo param with maybe_sudo method. #22

merged 1 commit into from
Jan 16, 2025

Conversation

theory
Copy link
Member

@theory theory commented Jan 10, 2025

The maybe_sudo method creates a new command based on whether sudo might be needed and, if so, whether pkglibdir is writable. If sudo is true and pkglibdir is not writable it creates a Command with sudo as the program and the program passed as its first argument. Otherwise it returns a Command with the passed program.

With that intelligence, remove the sudo param from the interface and all methods. There will be no need to specify sudo as a boolean, as the package can figure it out for itself.

In the future it might be necessary to allow the user to specify a path to sudo (and make and cargo and whatever else?), but for now it will be enough to have the user make sure that such commands are in the path.

Also remove mut from PgConfig::get, as it's unnecessary and makes its usage more difficult. Found while using it in maybe_sudo.

@theory theory requested a review from vrmiguel January 10, 2025 03:01
@theory theory self-assigned this Jan 10, 2025
The `maybe_sudo` method creates a new command based on whether `sudo`
might be needed and, if so, whether `pkglibdir` is writable. If `sudo`
is true and `pkglibdir` is not writable it creates a Command with `sudo`
as the program and the program passed as its first argument. Otherwise
it returns a Command with the passed program.

With that intelligence, remove the `sudo` param from the interface and
all methods. There will be no need to specify `sudo` as a boolean, as
the package can figure it out for itself.

In the future it might be necessary to allow the user to specify a path
to `sudo` (and `make` and `cargo` and whatever else?), but for now it
will be enough to have the user make sure that such commands are in the
path.

Also remove `mut` from `PgConfig::get`, as it's unnecessary and makes
its usage more difficult. Found while using it in `maybe_sudo`.
Copy link

codecov bot commented Jan 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.19%. Comparing base (8c20357) to head (ea8ff13).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #22      +/-   ##
==========================================
+ Coverage   98.18%   98.19%   +0.01%     
==========================================
  Files          10       10              
  Lines         495      499       +4     
==========================================
+ Hits          486      490       +4     
  Misses          9        9              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@theory theory merged commit ea8ff13 into main Jan 16, 2025
14 checks passed
@theory theory deleted the integrate-sudo branch January 16, 2025 20:25
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.

2 participants