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

venv: prepare for sh removal and add tests #50

Merged
merged 3 commits into from
Apr 10, 2023

Conversation

gotmax23
Copy link
Collaborator

@gotmax23 gotmax23 commented Apr 9, 2023

  • Deprecate the get_command methods on VenvRunner and FakeVenvRunner
  • Add new async_log_run and log_run methods to replace the former.
    These work the same as their counterparts in
    antsibull_core.subprocess_util, but the VenvRunner version enforces
    a clean environment and requires paths to be installed in the venv.

@gotmax23
Copy link
Collaborator Author

gotmax23 commented Apr 9, 2023

@felixfontein, can you take a cursory look before I clean this up and write a changelog frag? I'm not sure if you had something else in mind.

@felixfontein
Copy link
Collaborator

@gotmax23 looks good to me!

@gotmax23 gotmax23 force-pushed the new_venv branch 3 times, most recently from 25c86df to 28bb17e Compare April 9, 2023 14:50
@gotmax23 gotmax23 marked this pull request as ready for review April 9, 2023 14:55
@gotmax23 gotmax23 requested a review from felixfontein April 9, 2023 14:55
gotmax23 added 3 commits April 9, 2023 16:26
- Deprecate the `get_command` methods on VenvRunner and FakeVenvRunner
- Add new `async_log_run` and `log_run` methods to replace the former.
  These work the same as their counterparts in
  `antsibull_core.subprocess_util`, but the VenvRunner version enforces a
  clean environment and requires paths to be installed in the venv.
@felixfontein felixfontein merged commit 39a4356 into ansible-community:main Apr 10, 2023
@felixfontein
Copy link
Collaborator

@gotmax23 thanks for creating this!

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