Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for --no-use-platform-suffix. #96

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

jsirois
Copy link
Contributor

@jsirois jsirois commented Sep 12, 2024

No description provided.

Add support for `--no-use-platform-suffix` as a complement to `--use-platform-suffix`. Without
specifying either, auto-disambiguation is still used to add a platform suffix whenever the
set of target platforms is not just the current platform, but you can also now force a platform
suffix to never be used by specifying `--no-use-platform-suffix`.
Copy link
Contributor Author

@jsirois jsirois Sep 12, 2024

Choose a reason for hiding this comment

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

Pex needs this for the --scie-name-style platform-parent-dir option it added in pex-tool/pex#2523 to work right. In addition to the new test in this change, I used --scie-science-binary ... to double check this worked over there (to solve pex-tool/pex#2516) as well.

@@ -102,3 +103,10 @@ jobs:
files: dist/science-*
fail_on_unmatched_files: true
discussion_category_name: Announcements
- name: Add Binaries to ${{ needs.determine-tag.outputs.release-tag }} Release
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Windows machines were messing up the release notes; so I added this fork. We only need release notes and discussion creation to be triggered once since they are not unique per platform like the binaries are; so this makes sense anyhow.

@@ -542,14 +542,15 @@ def dest_dir_option():

def use_platform_suffix_option():
return click.option(
"--use-platform-suffix",
is_flag=True,
"--use-platform-suffix/--no-use-platform-suffix",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The docsite / this CLI help rendered here: https://jsirois.github.io/lift/cli.html#science-lift-build

@jsirois
Copy link
Contributor Author

jsirois commented Sep 13, 2024

I'm going to move this one along since its small and I'd like to both check the release fix and get Pex rolling along with its fix based on this new option. Consider this FYI.

@jsirois jsirois merged commit 75bcd2c into a-scie:main Sep 13, 2024
7 checks passed
@jsirois jsirois deleted the use-platform-suffix/tristate branch September 13, 2024 01:16
Copy link
Collaborator

@kwlzn kwlzn left a comment

Choose a reason for hiding this comment

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

lgtm

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