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

[RSDK-7007] Add Manifest Generation in CI #78

Merged
merged 11 commits into from
Mar 5, 2025
Merged

[RSDK-7007] Add Manifest Generation in CI #78

merged 11 commits into from
Mar 5, 2025

Conversation

Otterverse
Copy link
Member

Add manifest generation to agent builds, to support incoming work in App.

@Otterverse Otterverse requested a review from abe-winter March 4, 2025 16:48
@Otterverse Otterverse changed the title [RSDK-7007] Manifest [RSDK-7007] Add Manifest Generation in CI Mar 4, 2025
Copy link
Member

@abe-winter abe-winter left a comment

Choose a reason for hiding this comment

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

LGTM, guessing you will need to test e2e, also see my note about windows support in filenames

fi

# Set platform based on architecture in binary name
if [[ "$BINARY_NAME" == *"x86_64"* ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

I say this in the tone of the trailer for a horror movie but remember, windows is coming

linux/amd64 isn't the only amd64 anymore

I think you need to do one of:

  • handle *windows-x86_64 first, and make sure *x86_64 is an else case
  • always include OS in the filename

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call. I'd been trying not to do ANYTHING windows here, to not conflict with your PR, but obviously new stuff in CI wouldn't be in your PR anyway.

Updated this to handle OS and Arch separately, so windows should work.

Copy link
Member

Choose a reason for hiding this comment

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

you're assuming I haven't created an entire parallel manifest system on my branch that will lead to terrible merge conflicts

@Otterverse Otterverse merged commit 75deec7 into main Mar 5, 2025
3 checks passed
@Otterverse Otterverse deleted the manifest branch March 5, 2025 00:20
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