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.

Sign up for GitHub

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

Jump to bottom

The determineRuntime in polyglotpkg should not try to determine and change the runtime #391

Closed
Michaelpalacce opened this issue Aug 22, 2024 · 4 comments · Fixed by #456
Closed

The determineRuntime in polyglotpkg should not try to determine and change the runtime #391

Michaelpalacce opened this issue Aug 22, 2024 · 4 comments · Fixed by #456
Assignees
@Michaelpalacce
Labels
area/polyglotpkg Relates to polyglotpkg module or archetype kind/enhancement Enhancement or improvement of existing features lang/typescript Related to typescript code triage/accepted The issue was accepted and will be done version/major Introduces a breaking change
Milestone
3.x.x

Comments

@Michaelpalacce
Copy link
Collaborator

Description

There is code currently that checks if the user supplied something like: "node:12" in an abx action to translate that to "nodejs". This is a responsibility of the user. vRO and ABX actions are different in that regard and that is ok.
@Michaelpalacce Michaelpalacce added kind/enhancement Enhancement or improvement of existing features lang/typescript Related to typescript code version/major Introduces a breaking change triage/needed Needs to be discussed by project maintainers area/polyglotpkg Relates to polyglotpkg module or archetype labels Aug 22, 2024
@Michaelpalacce Michaelpalacce added this to the 3.0.0 milestone Aug 23, 2024
@VenelinBakalov
Copy link
Collaborator

VenelinBakalov commented Sep 3, 2024 •
edited
Loading

For 2.x we keep the default behaviour as it currently is.

With 3.0.0 we need to remove the wrongly defined aria sample as well: https://github.com/vmware-samples/build-tools-for-vmware-aria-samples/blob/main/polyglot-all/src/polyglotallnodejs/polyglot.json
At the time of creation it was properly defined since the initial versions only supported nodejs as runtime but that changed later on.

@VenelinBakalov VenelinBakalov added triage/accepted The issue was accepted and will be done and removed triage/needed Needs to be discussed by project maintainers labels Sep 10, 2024
@VenelinBakalov
Copy link
Collaborator

Related code change:
https://github.com/vmware/build-tools-for-vmware-aria/blob/main/typescript/polyglotpkg/src/lib/utils.ts#L37-L78

@pe1pip

This comment was marked as off-topic.

Sign in to view
@Michaelpalacce
Copy link
Collaborator Author

@pe1pip Valid comment, but this is not the place for this, open an issue if one does not exist.

@Michaelpalacce Michaelpalacce linked a pull request Oct 16, 2024 that will close this issue
[polyglotpkg] (#391) ABX runtimes no longer default to vRO ones #456
Merged
9 tasks
@Michaelpalacce Michaelpalacce mentioned this issue Oct 16, 2024
Merged
9 tasks
@Michaelpalacce Michaelpalacce self-assigned this Oct 16, 2024
@Michaelpalacce Michaelpalacce modified the milestones: BTVA Next, 3.0.0 Nov 1, 2024
Michaelpalacce added a commit that referenced this issue Nov 1, 2024
@Michaelpalacce
Merge pull request #456 from vmware/refactor/391-determine-runtime-sh…
f097213 
…ould-not-try-to-change-the-runtime

[polyglotpkg] (#391) `ABX` runtimes no longer default to `vRO` ones
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Michaelpalacce Michaelpalacce

Labels
area/polyglotpkg Relates to polyglotpkg module or archetype kind/enhancement Enhancement or improvement of existing features lang/typescript Related to typescript code triage/accepted The issue was accepted and will be done version/major Introduces a breaking change
Projects
None yet
Milestone
3.x.x
Development

Successfully merging a pull request may close this issue.

[polyglotpkg] (#391) ABX runtimes no longer default to vRO ones vmware/build-tools-for-vmware-aria
3 participants
@Michaelpalacce @VenelinBakalov @pe1pip