Skip to content

Add architecture option#32

Merged
maxday merged 2 commits into
mainfrom
maxday/add-option-archi
Oct 25, 2021
Merged

Add architecture option#32
maxday merged 2 commits into
mainfrom
maxday/add-option-archi

Conversation

@maxday

@maxday maxday commented Oct 25, 2021

Copy link
Copy Markdown
Contributor

This PRs adds the architecture option to enable faster build while developing

@maxday
maxday requested a review from a team as a code owner October 25, 2021 15:59
elif [ $ARCHITECTURE == "arm64" ]; then
echo "Building for arm64 only"
docker_build_zip arm64
else

@nhinsch nhinsch Oct 25, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

arn64 -> arm64

@nhinsch

nhinsch commented Oct 25, 2021

Copy link
Copy Markdown
Contributor

I would add a comment at the top of the build_binary_and_layer_dockerized.sh file with instructions/a usage example so users can see the different arguments and what they do.

@nhinsch nhinsch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, just a couple changes needed.

I noticed the integration tests are failing but that's probably worth fixing in a different PR.

@maxday
maxday merged commit 99acbf1 into main Oct 25, 2021
@maxday
maxday deleted the maxday/add-option-archi branch October 25, 2021 18:45
echo "Building for arm64 only"
docker_build_zip arm64
else
echo "No valid ARCHITECTURE variable found (allowed values are : arm64, amd64), building for both amd64 and arm64"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to build both? The default in this case would be amd64. Is this just in case a customer changes it later?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah we need to build it for both architectures since this script is used during our release process

litianningdatadog pushed a commit that referenced this pull request Dec 1, 2025
route was changed, as opposed to schema version
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.

3 participants