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

Improve warning message for missing ImageOS env. variable (self-hosted runners) #59

Merged
merged 1 commit into from
Jul 12, 2021
Merged

Improve warning message for missing ImageOS env. variable (self-hosted runners) #59

merged 1 commit into from
Jul 12, 2021

Conversation

paulo-ferraz-oliveira
Copy link
Collaborator

Closes #58.

}[process.env.ImageOS]
}

function getContainerFromOS() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is where the guessing game now happens. It's convoluted, sure, but I'm not sure there's a better way to fetch this info. from the target OSs

Copy link
Member

Choose a reason for hiding this comment

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

See my comment here #58 (comment)

Copy link
Collaborator Author

@paulo-ferraz-oliveira paulo-ferraz-oliveira Jul 12, 2021

Choose a reason for hiding this comment

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

Per "I'd rather us not start doing guess work around operating systems" I'm reverting this guessing bit. Whatever it is, the self-hosted runner will have to use the ImageOS env. variable to achieve the same behaviour (as suggested by @ericmj). I surely don't want to introduce a new env. variable and that one's already documented in the README.

@paulo-ferraz-oliveira paulo-ferraz-oliveira changed the title Fix action for self-hosted runners Improve warning message for missing ImageOS env. variable (self-hosted runners) Jul 12, 2021
return ImageOSToContainer[process.env.ImageOS]
if (!containerFromEnvImageOS) {
throw new Error(
"Tried to map a target OS from env. variable 'ImageOS', but failed. If you're using a " +
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Example output:

Tried to map a target OS from env. variable 'ImageOS', but failed. If you're using a self-hosted runner, you should set 'env': 'ImageOS': ... to one of the following: ['ubuntu16', 'ubuntu18', 'ubuntu20', 'win16', 'win19']

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(this way the list will be as updated as the constant definition)

@paulo-ferraz-oliveira paulo-ferraz-oliveira merged commit 9283f85 into erlef:main Jul 12, 2021
@paulo-ferraz-oliveira paulo-ferraz-oliveira deleted the fix/for-self-hosted-runners branch July 12, 2021 17:35
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.

Action incompatible with self-hosted runners
2 participants