Skip to content

Conversation

@josecorella
Copy link
Contributor

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@josecorella josecorella closed this Jul 2, 2025
@josecorella josecorella reopened this Jul 2, 2025
@josecorella josecorella marked this pull request as ready for review July 2, 2025 19:11
@josecorella josecorella requested a review from a team as a code owner July 2, 2025 19:11
Copy link

@lucasmcdonald3 lucasmcdonald3 left a comment

Choose a reason for hiding this comment

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

It's cool that this is now totally native and doesn't shell out to CodeBuild at all.

Do you want to clean up the CodeBuild .yml in this PR or a different one?

@josecorella
Copy link
Contributor Author

It's cool that this is now totally native and doesn't shell out to CodeBuild at all.

Do you want to clean up the CodeBuild .yml in this PR or a different one?

I'll clean up in a different pr

Comment on lines +57 to +60
- name: Install python version specific dependencies
if: matrix.python-version == '3.12'
run: |
pip install -r dev_requirements/ci-requirements.txt

Choose a reason for hiding this comment

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

I don't fully understand this one -- why just 3.12?

Ditto in the examples file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for some reason it installs these in the cb job, since python 3.12 requires setuptools and the other versions dont

Copy link

@lucasmcdonald3 lucasmcdonald3 Jul 2, 2025

Choose a reason for hiding this comment

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

Oh! Thanks, I remember this now.

Could you make a comment just above this saying something like

# Python no longer bundles setuptools starting in 3.12

just so we know we'll have to bump this once we add 3.13 support.

Ideally we'd write

matrix.python-version >= '3.12'

but I don't think that works

@josecorella josecorella merged commit 7c07105 into master Jul 2, 2025
85 checks passed
@josecorella josecorella deleted the jocorell/move-cb-ci-jobs-to-gha branch July 2, 2025 22:28
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