Skip to content

Conversation

@michaeltryby
Copy link
Collaborator

This pull request updates the build and test workflows to improve compatibility with newer Python versions and macOS environments, and to streamline artifact handling. The most important changes are grouped below by theme.

Python and GitHub Actions Version Updates:

  • Upgraded actions/checkout and actions/setup-python to use version 5 in all workflows for improved reliability and future compatibility. [1] [2]
  • Added support for Python 3.13 (cp313) in wheel builds and cross-wheel builds, while removing support for Python 3.8 (cp38). [1] [2] [3]

macOS Build Environment Improvements:

  • Set MACOSX_DEPLOYMENT_TARGET to "11.0" in both wheel and unit test workflows to ensure compatibility with newer macOS versions. [1] [2]
  • Removed explicit deployment target from CMake arguments for macOS builds in setup.py, relying on environment configuration instead.

Build Matrix and Architecture Updates:

  • Expanded macOS build architectures to include both x86_64 and arm64 for broader compatibility, and updated artifact naming for clarity.
  • Updated Linux and macOS architecture and skip lists to streamline builds and avoid unnecessary configurations. [1] [2]

Artifact Upload and Naming Improvements:

  • Improved artifact upload steps by adding descriptive names to wheel artifacts for easier identification and management. [1] [2] [3]

General Workflow Streamlining:

  • Simplified workflow configuration by removing unused or redundant options, such as the os matrix in cross-wheel builds.

These updates help ensure the project is ready for newer Python and macOS releases, improve CI reliability, and make build artifacts easier to manage.

Remove unnecessary echo command for PWD.
Removed step to get wheel filename and set a static name for the uploaded artifact.
Remove deployment target specification for macOS in setup.py.
Set MACOSX_DEPLOYMENT_TARGET environment variable for building wheel.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request modernizes the build and test workflows by upgrading to newer versions of GitHub Actions, adding Python 3.13 support while dropping Python 3.8, and improving macOS compatibility through environment-based deployment target configuration.

  • Updated GitHub Actions to version 5 and added Python 3.13 support while removing Python 3.8
  • Configured macOS deployment target through environment variables instead of CMake arguments
  • Enhanced build matrix to include ARM64 architecture and improved artifact naming

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
.github/workflows/build_wheel.yml Updated Actions versions, Python support matrix, macOS environment configuration, and artifact naming
.github/workflows/unit_test.yml Added macOS deployment target environment variable
swmm-toolkit/setup.py Removed hardcoded macOS deployment target from CMake arguments

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

uses: actions/checkout@v3
with:
submodules: true
uses: actions/checkout@v5
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The checkout action is missing the submodules: true configuration that was present in the original version. This could cause build failures if the project depends on git submodules.

Suggested change
uses: actions/checkout@v5
uses: actions/checkout@v5
with:
submodules: true

Copilot uses AI. Check for mistakes.

- uses: actions/upload-artifact@v4
with:
name: wheels-linux-aarch64-${{ matrix.pyver }}
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

There's an extra space after 'name:' in the artifact name. Should be name: wheels-linux-aarch64-${{ matrix.pyver }}

Suggested change
name: wheels-linux-aarch64-${{ matrix.pyver }}
name: wheels-linux-aarch64-${{ matrix.pyver }}

Copilot uses AI. Check for mistakes.
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.

1 participant