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

re-adding linux/ppc64le support #173

Merged
merged 3 commits into from
Aug 2, 2022

Conversation

adilhusain-s
Copy link
Contributor

  • re-adding support for linux/ppc64le arch
  • addressing the issue 170

please review and merge.

@adilhusain-s adilhusain-s marked this pull request as ready for review July 22, 2022 12:56
@adilhusain-s adilhusain-s requested review from a team and jpkrohling July 22, 2022 12:56
@@ -27,7 +27,7 @@ jobs:
- name: Setup QEMU
uses: docker/setup-qemu-action@v2
with:
platforms: arm64
platforms: arm64,ppc64le
Copy link
Member

Choose a reason for hiding this comment

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

Can you also add in the readme, or in another place you find suitable, what needs to be done to add new platforms? This problem has happened already in the past with other platforms, and I think we could have prevented it if we had clear instructions on how to add new platforms.

@adilhusain-s
Copy link
Contributor Author

@jpkrohling
How about adding it to contributing.md?

@jpkrohling
Copy link
Member

That would work, I believe

@adilhusain-s
Copy link
Contributor Author

@jpkrohling
does this look good?

ADDING support for new platform/architecture

  • build-test opentelemetry-collector and opentelemetry-collector-ccontrib locally.
  • add support for opentelemetry-colector and opentelemetry-collector-contrib in their respective repository.
  • Add platform/arch in goreleaser/configure.go.
  • Regenerate the .goreleaser.yml:
make generate-goreleaser
  • In setup qemu github action, add the platform architecture in .github/workflows/ci-goreleaser.yaml and release.yaml
  • Raised the PR with changes.

@jpkrohling
Copy link
Member

Yes, that's a good start. If we need, we can improve this doc in next iterations.

@adilhusain-s
Copy link
Contributor Author

@jpkrohling
Please review and merge.

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@@ -63,3 +63,13 @@ This is accomplished by installing [qemu](https://www.qemu.org/), and then [enab
apt-get install qemu binfmt-support qemu-user-static
docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
```

### Adding support for new platforms or architectures
Copy link
Member

Choose a reason for hiding this comment

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

I reworded this a bit. Please take a look and give your approval. Once it's done, I think this is ready to be merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks good to me.

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