Skip to content

♻️ refactor: hard written target_arch、target_os_info#14

Merged
gouzil merged 3 commits intomainfrom
reconfiguration/platform_tag_arch
Apr 15, 2025
Merged

♻️ refactor: hard written target_arch、target_os_info#14
gouzil merged 3 commits intomainfrom
reconfiguration/platform_tag_arch

Conversation

@gouzil
Copy link
Collaborator

@gouzil gouzil commented Apr 15, 2025

硬编 target_arch、target_os_info,key 是 pypi 发布的 tag

@gouzil gouzil requested a review from Copilot April 15, 2025 16:10
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 PR refactors how target architecture and OS information is handled, aligning it with the PyPI tag requirements and updating the related build and release configurations.

  • Update version in pyproject.toml from "0.16.0-alpha.0" to "0.16.0-alpha.1".
  • Refactor target detection in hatch_build.py by using a BUILD_TARGET mapping instead of automated platform detection.
  • Update GitHub Actions workflows in release.yaml to utilize the new platform tag and architecture fields.

Reviewed Changes

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

File Description
pyproject.toml Version update for release.
hatch_build.py Refactored target_arch and target_os_info retrieval and validation using BUILD_TARGET mapping.
.github/workflows/release.yaml CI workflow matrix updated to use new platform_tag and arch fields.

Comment on lines +43 to +44
assert target_arch is not None, f"CIBW_ARCHS not set see: {BUILD_TARGET}"
assert target_os_info is not None, f"CIBW_PLATFORM not set see: {BUILD_TARGET}"
Copy link

Copilot AI Apr 15, 2025

Choose a reason for hiding this comment

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

Using 'assert' for checking environment variables may lead to it being bypassed in optimized mode; consider replacing it with an explicit condition that raises a proper exception.

Suggested change
assert target_arch is not None, f"CIBW_ARCHS not set see: {BUILD_TARGET}"
assert target_os_info is not None, f"CIBW_PLATFORM not set see: {BUILD_TARGET}"
if target_arch is None:
raise RuntimeError(f"CIBW_ARCHS not set. See: {BUILD_TARGET}")
if target_os_info is None:
raise RuntimeError(f"CIBW_PLATFORM not set. See: {BUILD_TARGET}")

Copilot uses AI. Check for mistakes.
Comment on lines +43 to +44
assert target_arch is not None, f"CIBW_ARCHS not set see: {BUILD_TARGET}"
assert target_os_info is not None, f"CIBW_PLATFORM not set see: {BUILD_TARGET}"
Copy link

Copilot AI Apr 15, 2025

Choose a reason for hiding this comment

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

Replace the 'assert' with a direct check and explicit exception to ensure these environment variables are always validated in production builds.

Suggested change
assert target_arch is not None, f"CIBW_ARCHS not set see: {BUILD_TARGET}"
assert target_os_info is not None, f"CIBW_PLATFORM not set see: {BUILD_TARGET}"
if target_arch is None:
raise RuntimeError(f"Environment variable 'CIBW_ARCHS' is not set. Expected one of: {list(BUILD_TARGET.keys())}")
if target_os_info is None:
raise RuntimeError(f"Environment variable 'CIBW_PLATFORM' is not set. Expected one of: {list(BUILD_TARGET.keys())}")

Copilot uses AI. Check for mistakes.
@gouzil gouzil merged commit 5f6170b into main Apr 15, 2025
11 checks passed
@gouzil gouzil deleted the reconfiguration/platform_tag_arch branch April 15, 2025 16:21
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