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.

Sign up for GitHub

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

Jump to bottom

Add PowerShell support #2749

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Add PowerShell support #2749

wants to merge 4 commits into from

Conversation

fblanchetNaN
Copy link

@fblanchetNaN fblanchetNaN commented Jul 28, 2023 •
edited
Loading

Make sure you have checked all steps below.

Prerequisite

  • Please consider implementing the feature as a hook script or plugin as a first step.
    • pyenv has some powerful support for plugins and hook scripts. Please refer to Authoring plugins for details and try to implement it as a plugin if possible.
  • Please consider contributing the patch upstream to rbenv, since we have borrowed most of the code from that project.
    • We occasionally import the changes from rbenv. In general, you can expect changes made in rbenv will be imported to pyenv too, eventually.
    • Generally speaking, we prefer not to make changes in the core in order to keep compatibility with rbenv.
  • My PR addresses the following pyenv issue (if any)
    • N/A

Description

  • Here are some details about my PR

I added support for PowerShell (on Linux), it could probably be implemented for rbenv also, however I never used ruby and don't feel confident to implement it.

I know it's maybe a patch too important for the core code, if so just close this (draft) PR.

Tests

  • My PR adds the following unit tests (if any): WIP

I don't know how BATS works, I need help / hints to understand what should I do -- where is the tested shell specified?

@native-api
Copy link
Member

I don't know how BATS works, I need help / hints to understand what should I do -- where is the tested shell specified?

Tests can be run with make test. It shows the commands it runs so you can run them by hand or run bats with other arguments when diagnosing tests (you'll probably need -f) etc.

Look at the existing tests for the files you've changed to get an idea of what they do. They're basically Bash scripts with some extra syntax that denotes tests and setup/teardown code that's preprocessed and then executed by Bats.

We're using a rather old Bats version, 1.2.0, the docs are at https://github.com/bats-core/bats-core/blob/v1.2.0/README.md . Apart from functions defined by Bats, we use additional ones defined in test_helper.bash and test files themselves.

Most tests are unit tests that mock any external interactions; IIRC in the affected files, there're only 1-2 integration tests for Fish.

@native-api
Copy link
Member

I know it's maybe a patch too important for the core code, if so just close this (draft) PR.

@pyenv/pyenv-core-maintainers IMO supporting PowerShell is fine. However, due to unfamiliarity with it, we may need to request help if more stuff interacting with it will be needed in the future or problems are reported.

@native-api native-api closed this Dec 26, 2023
@native-api native-api reopened this Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fblanchetNaN @native-api