Skip to content

Use the powershell activate script on windows#13

Merged
shsms merged 1 commit intofrequenz-floss:v1.x.xfrom
shsms:windows-support
Nov 19, 2025
Merged

Use the powershell activate script on windows#13
shsms merged 1 commit intofrequenz-floss:v1.x.xfrom
shsms:windows-support

Conversation

@shsms
Copy link
Copy Markdown
Contributor

@shsms shsms commented Nov 17, 2025

No description provided.

@shsms shsms requested a review from a team as a code owner November 17, 2025 17:23
@github-actions github-actions Bot added part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) part:action Affects the action itself labels Nov 17, 2025
@shsms shsms requested a review from llucax November 17, 2025 17:24
@shsms
Copy link
Copy Markdown
Contributor Author

shsms commented Nov 18, 2025

I will test this first and report here.

Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
@shsms
Copy link
Copy Markdown
Contributor Author

shsms commented Nov 19, 2025

Copy link
Copy Markdown
Contributor

@llucax llucax left a comment

Choose a reason for hiding this comment

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

Approving in case you don't want to go through another round of testing, I think it is OK to keep it as it is, but the alternative approach I suggest would make the pipeline less noisy, as we don't get a few skipped jobs.

Also at some point it would probably be nice to add some sort of CI, but I guess it doesn't happen often that we need changes for this...

Comment thread action.yaml
if: runner.os != 'Windows'
shell: bash
env:
NOX_SESSION: ${{ inputs.nox-session }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
NOX_SESSION: ${{ inputs.nox-session }}
ACTIVATE_SCRIPT: ${{ runner.os == 'Windows' && 'Scripts/Activate.ps1' || `bin/activate` }}

Comment thread action.yaml
env:
NOX_SESSION: ${{ inputs.nox-session }}
run: |
. ".nox/$NOX_SESSION/bin/activate"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
. ".nox/$NOX_SESSION/$ACTIVATE_SCRIPT"

Comment thread action.yaml
Comment on lines +84 to +92
- name: Print pip freeze for nox venv (debug) (Windows)
if: runner.os == 'Windows'
shell: pwsh
env:
NOX_SESSION: ${{ inputs.nox-session }}
run: |
. ".nox/$env:NOX_SESSION/Scripts/Activate.ps1"
pip freeze

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Print pip freeze for nox venv (debug) (Windows)
if: runner.os == 'Windows'
shell: pwsh
env:
NOX_SESSION: ${{ inputs.nox-session }}
run: |
. ".nox/$env:NOX_SESSION/Scripts/Activate.ps1"
pip freeze

@shsms
Copy link
Copy Markdown
Contributor Author

shsms commented Nov 19, 2025

That won't be enough, because on windows, that script is for PowerShell. So it would need more conditional checks and more trial and error.

Would leave for later.

@shsms shsms added this pull request to the merge queue Nov 19, 2025
@shsms
Copy link
Copy Markdown
Contributor Author

shsms commented Nov 19, 2025

Release as v1.1 or v2?

Merged via the queue into frequenz-floss:v1.x.x with commit e1351cf Nov 19, 2025
2 checks passed
@shsms shsms deleted the windows-support branch November 19, 2025 15:45
@shsms
Copy link
Copy Markdown
Contributor Author

shsms commented Nov 19, 2025

as we don't get a few skipped jobs.

Also, they are steps in a job, not jobs themselves. And I didn't see any skipped logs.

@llucax
Copy link
Copy Markdown
Contributor

llucax commented Nov 19, 2025

v1.1, there are no breaking changes

@llucax llucax linked an issue Mar 19, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:action Affects the action itself part:tooling Affects the development tooling (CI, deployment, dependency management, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Action does not work for Windows

2 participants