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

added os_version to unix build macro #116

Conversation

shimwell
Copy link
Member

This PR into a feature branch is one option for getting os_version from the github action into the unix_share_build script as suggested in the review for #115

The only down side I can see with this approach is that the unix_share_build script currently makes use of lsb_release to find the Linux version. So perhaps this should also be removed?

Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

This seems like a good plan to me - just one suggestion for an improvement of an inherited issue...

scripts/unix_share_build.sh Outdated Show resolved Hide resolved
@gonuke
Copy link
Member

gonuke commented Aug 10, 2021

I think it makes sense to rely on the OS_VERSION variable instead of the lsb_release command, but I wonder how we avoid an inconsistency?

@shimwell
Copy link
Member Author

shimwell commented Aug 10, 2021

I think it makes sense to rely on the OS_VERSION variable instead of the lsb_release command, but I wonder how we avoid an inconsistency?

We could use lsb_release to check the user specified / github action specified argument. If there is disagreement then the script could raise an error

I was also half thinking of having os_minor_version and os_major_version in the github action but that can wait for iteration 2 :-)

@shimwell
Copy link
Member Author

Looks like we have settled on an option for this, so I shall merge it into the other feature branch so that the CI can be triggered

@shimwell shimwell merged commit 24534f2 into building_ubuntu_and_debian_inside_docker Aug 10, 2021
@shimwell shimwell deleted the option_1_for_getting_os_version branch August 10, 2021 12:05
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