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

Tweak and document disabling Docker build integration #5365

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

twz123
Copy link
Member

@twz123 twz123 commented Dec 16, 2024

Description

Don't default EMBEDDED_BINS_BUILDMODE to none when disabling Docker integration. This makes it clear that the binaries can't be built without it, at the moment. Inline the K0S_BUILD_IMAGE_FILE variable and replace it with the more generic GO_ENV_REQUISITES. Use the empty string to disable Docker integration: The previous sentinel value of "false" may have given the impression that "true" is the correct value to enable Docker build integration. Also, "false" is actually an executable provided by coreutils. An empty string can never point to a potentially executable file, and won't give the impression that DOCKER is some sort of boolean variable.

Document the new build variables in the README.

Follow-up to:

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

How Has This Been Tested?

  • Manual test
  • Auto test added

Checklist:

  • My code follows the style guidelines of this project
  • My commit messages are signed-off
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

@twz123 twz123 added the chore label Dec 16, 2024
@twz123 twz123 added this to the 1.32 milestone Dec 16, 2024
@twz123 twz123 marked this pull request as ready for review December 16, 2024 15:47
@twz123 twz123 requested review from a team as code owners December 16, 2024 15:47
@twz123 twz123 requested review from ncopa and makhov December 16, 2024 15:47
Copy link
Contributor

This pull request has merge conflicts that need to be resolved.

@twz123 twz123 force-pushed the make-disable-docker-integration branch 2 times, most recently from 094ccbd to be8b5e6 Compare December 19, 2024 10:30
Copy link
Contributor

This pull request has merge conflicts that need to be resolved.

The previous sentinel value of "false" may have given the impression
that "true" is the correct value to enable Docker build integration.
Also, "false" is actually an executable provided by coreutils. An empty
string can never point to a potentially executable file, and won't give
the impression that DOCKER is some sort of boolean variable.

Signed-off-by: Tom Wieczorek <[email protected]>
Don't default EMBEDDED_BINS_BUILDMODE to none when disabling Docker
integration. This makes it clear that the binaries can't be built
without it, at the moment. Inline the K0S_BUILD_IMAGE_FILE variable and
replace it with the more generic GO_ENV_REQUISITES. Document the new
variable in the README.

Signed-off-by: Tom Wieczorek <[email protected]>
@twz123 twz123 force-pushed the make-disable-docker-integration branch from be8b5e6 to 8ff47f6 Compare December 23, 2024 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant