-
Notifications
You must be signed in to change notification settings - Fork 440
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
[Request for Maintenance] CPP tooling in open-telemetry/build-tools #2550
Comments
I support the idea of new repository with the possibility to publish docker image. |
Hi @jsuereth I support this idea also. |
I think it was earlier discussed and agreed to remove this tooling - #2050 |
I support this idea of moving |
It is correct the opentelemetry-cpp CI does not use the docker image provided, which is why the decision then was to remove it. Since then, I had to reinstall a new computer from 0, and discovered how actually useful the docker image is to format all the code, especially for cmake-format, when all the tooling is not available in the distribution used. For new contributors submitting a PR for the first time, clang-format and cmake-format are a strong barrier. Keeping the docker image available, assuming we advertise it more, will help. In short, I am reverting my previous opinion, and now prefer to keep the tooling. |
Got it, thanks. makes sense to have a separate repo that publishes a docker image of this tooling. |
@marcalff Should I assume you'd be the primary owner then helping me move this tooling into its own repository? I can set up the repository with all existing CPP maintainers/approvers and help get the docker-image publishing working, then let you take over from there, if this all makes sense. Accepting this issue (triage/accepted) label will tell me to go forward and I'll start creating the repository. |
Yes, agreed. |
@marcalff I'm still working on getting your dockerhub publishing credentials set up, but please try out the repository here: https://github.com/open-telemetry/cpp-build-tools |
When you're comfortable with new repo, please approve: open-telemetry/build-tools#275 Your new repo should be able to build docker images from main every time you merge a commit. |
Thanks @jsuereth Is there a way for cpp-maintainers to get admin privileges on this new repo ? We need to setup branch protection rules, enforce easyCLA, etc. |
I added DOCKER_USERNAME/DOCKER_PASSWORD repository secrets to https://github.com/open-telemetry/cpp-build-tools/settings/secrets/actions You should be able to use them in the actions. |
@marcalff Branch protection rules and easyCLA should already be set up for you (TC does that when creating any repository for consistency) Thanks @tigrannajaryan for setting up the secrets!!! |
no easyCLA check in the repo open-telemetry/cpp-build-tools#4 |
EasyCLA is configured at the organization level to be on for all repositories, let me ask around I may ahve missed a check in the repo setup that's preventing it from commenting: https://github.com/open-telemetry/community/blob/main/docs/how-to-configure-new-repository.md |
The team https://github.com/orgs/open-telemetry/teams/cpp-maintainers/repositories has maintain privilege, this part seems ok. The team https://github.com/orgs/open-telemetry/teams/cpp-approvers/repositories only has the triage privilege, this should be write instead. |
@marcalff fixed. |
I'm going to close out this issue. Thank you for taking the new repository! Please open bugs in Thank you again! |
There's a lot of dependabot failures to upgrade the cpp tooling in build-tools repository.
As such, I'd like to propose the following:
cpp-build-tools
repository. This would publish a docker image just like the existing build-tools repo.Today - Given the distributed nature of ownership of the
build-tools
repository, it's hard and/or impractical for CPP maintainers to make fixes to build-tools, get reviews and update their projects.I'm unfortunately unable to attend your SiG due to conflicting meetings, but am happy to discuss the process of passing maintenance of these tools to the C++ maintainers on this issue or (if necessary) in the specification meeting.
Related: open-telemetry/opentelemetry-specification#3899
Related: open-telemetry/semantic-conventions#767
The text was updated successfully, but these errors were encountered: