Skip to content

Conversation

drbergman
Copy link
Collaborator

No description provided.

@drbergman drbergman requested a review from Copilot April 4, 2025 14:26
Copy link

@Copilot 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.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

.github/workflows/build_binaries.yml:252

  • [nitpick] There is an inconsistency between the naming of the final universal binary (using 'matrix.projects.binary') and the individual binaries (using 'matrix.projects.short_name'). Consider using a consistent naming scheme for clarity.
lipo -create -output ${{ matrix.projects.binary }} ${{ matrix.projects.short_name }}_macos13 ${{ matrix.projects.short_name }}_macosm1

@drbergman
Copy link
Collaborator Author

should've described this when I made it, but basically the handling of the MacOS binaries being split into multiple steps with intermediates cached meant we needed those intermediates to have unique names between the compiled projects. We previously used the binary field of the matrix which is project for most. This led to overwriting and thus the eventual assets sometimes claiming to be one project, but actually another. This fixes that by using the unique short_name to save intermediates.

@drbergman
Copy link
Collaborator Author

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)
.github/workflows/build_binaries.yml:252

  • [nitpick] There is an inconsistency between the naming of the final universal binary (using 'matrix.projects.binary') and the individual binaries (using 'matrix.projects.short_name'). Consider using a consistent naming scheme for clarity.
lipo -create -output ${{ matrix.projects.binary }} ${{ matrix.projects.short_name }}_macos13 ${{ matrix.projects.short_name }}_macosm1

This nitpick would be undesirable as it would mean that each compiled project would un-tar to reveal an executable named by the short_name. I suppose there's not a great reason to have project-specific executable names, but the current short names are not good for this across the board. E.g., rules would be a bad executable name.

@drbergman drbergman self-assigned this May 22, 2025
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