Visitar URL original
Add tk installation option by sco1 · Pull Request #144 · deadsnakes/action · GitHub
Skip to content

Add tk installation option - #144

Merged
asottile merged 1 commit into
deadsnakes:mainfrom
sco1:test-me-tk
Sep 7, 2024
Merged

asottile merged 1 commit into
deadsnakes:mainfrom
sco1:test-me-tk

Conversation

@sco1

@sco1 sco1 commented Sep 5, 2024 •

Copy link
Copy Markdown
Contributor

Not sure what you'd like the testing matrix to look like but I think this appears to work (test run, live test).

- {python: '3.7', debug: true, nogil: false}
- {python: '3.12-dev', debug: false, nogil: false}
- {python: '3.13-dev', debug: false, nogil: true}
- {python: '3.13-dev', debug: false, nogil: false, tk: true}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would just add one more, there's no need to test the cross product

Comment thread .github/workflows/main.yml Outdated
tk: ${{ matrix.tk }}

- name: check tk
if: ${{ matrix.tk }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the brackets aren't needed inside if:

Comment thread bin/install-python Outdated
if args.tk:
if args.debug:
packages.append(f'{py}-tk-dbg')
if args.nogil:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should be elif

Comment thread action.yml
steps:
- name: add deadsnakes ppa and install ${{ inputs.python-version }} ${{ inputs.debug == 'true' && '(debug)' || '' }}
run: ${{ github.action_path }}/bin/install-python ${{ inputs.python-version }} ${{ inputs.debug == 'true' && '--debug' || '' }} ${{ inputs.nogil == 'true' && '--nogil' || '' }}
- name: add deadsnakes ppa and install ${{ inputs.python-version }} ${{ inputs.debug == 'true' && '(debug)' || '' }} ${{ inputs.tk == 'true' && '(tk)' || '' }}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you want nogil in here too?

@sco1 sco1 mentioned this pull request Sep 6, 2024

@asottile asottile left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@asottile
asottile merged commit b4fb859 into deadsnakes:main Sep 7, 2024
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