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 output to match build system #3430

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ChrisBlankDe
Copy link
Contributor

github actions and azure pipelines use different environment variables we could use to identify the running system.
we should use this possibility to output formatting commands for the given system.

@ChrisBlankDe ChrisBlankDe requested a review from a team as a code owner March 19, 2024 08:14
@freddydk
Copy link
Contributor

I don't think testing for various environment variables deep inside ContainerHelper is the right approach.
I would accept a PR with 2 new properties in ContainerHelper config called IsGitHubActions and IsAzureDevOps - both default to $false. Then I would have two functions in HelperFunctions called WriteGroupStart and WriteGroupEnd with a message parameter, which would check for the two settings.
also - all places in Run-AlPipeline, where a group start is called - this function should be used and the value of githubActions in Run-AlPipeline should be defaulted to the setting - and the value of the parameter should be written into the bccontainerhelperconfig.IsGitHubActions (for that session).

@ChrisBlankDe
Copy link
Contributor Author

i wanted to change as little as possible, hence the somewhat inelegant solution. your approach is of course better.

but why shouldn't we automatically recognize the build system when loading the bccontainerhelper (possibly overridable by a setting)? if it only works nicely when the user activates a setting, most people will overlook it and not have the best experience.

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.

None yet

2 participants