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

Initial folder layout to allow further commits #3

Merged
merged 6 commits into from Mar 1, 2021
Merged

Conversation

maxgolov
Copy link
Contributor

Initial folder layout to allow further commits. I intentionally left a few changes as TBD to allow for additional flexibility:

  • coding style
  • code ownership
  • formatting rules

Different contributing individuals and companies should be able to use their own applicable coding style, i.e. if a module enables integration with a different system or product, it would most likely follow the coding style rules of the product.

I am not sure 100% re. the license, but keeping it Apache License Version 2.0 as the main repo. We may need to add an additional provision that optional community-contributed projects may each be under their own license?

@maxgolov maxgolov requested review from a team January 25, 2021 22:32
@maxgolov maxgolov linked an issue Jan 25, 2021 that may be closed by this pull request
Adjust .NET to C++ (correct copy-paste error of a similar dotnet document)
README.md Outdated Show resolved Hide resolved
Co-authored-by: Reiley Yang <reyang@microsoft.com>
reyang
reyang previously requested changes Jan 25, 2021
Copy link
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

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

Need to figure out who should be the maintainers of this project.

@maxgolov
Copy link
Contributor Author

maxgolov commented Jan 25, 2021

Need to figure out who should be the maintainers of this project.

It's not that we have a huge line of maintainers in this repo at this point, so I mirrored what was done for OpenTelemetry .NET : listing same contributors and maintainers as for the main C++ repository. I'd be glad to be one of the maintainers.

@maxgolov maxgolov requested review from a team January 25, 2021 22:39
help with reviews and code approval. However, as individual components are
developed by numerous contributors, approvers and maintainers are not expected
to directly contribute to every component.

Copy link
Member

@lalitb lalitb Jan 26, 2021

Choose a reason for hiding this comment

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

Probably, we can bring more clarity ( in another PR ) on role of approvers and maintainers in PR review. I like the way it is done for opentelemetry-collector-contrib (https://github.com/open-telemetry/opentelemetry-collector-contrib):
News PRs will be automatically associated with the reviewers based on CODEOWNERS. PRs will be also automatically assigned to one of the maintainers or approvers for facilitation. The facilitator is responsible for helping the PR author and reviewers to make progress or if progress cannot be made for closing the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's do this in a follow-up PR

Copy link
Member

Choose a reason for hiding this comment

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

I think we want to follow what folks are doing here https://github.com/open-telemetry/opentelemetry-dotnet-contrib#support.
I don't think we should assume the opentelemetry/cpp-maintainers and opentelemetry/cpp-approvers are the right group for this contrib repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the notion of opentelemetry/cpp-maintainers and opentelemetry/cpp-approvers, and added a reference to the CODEOWNERS process instead.

@lalitb lalitb requested a review from reyang February 24, 2021 19:01
@lalitb
Copy link
Member

lalitb commented Feb 24, 2021

Need to figure out who should be the maintainers of this project.

It's not that we have a huge line of maintainers in this repo at this point, so I mirrored what was done for OpenTelemetry .NET : listing same contributors and maintainers as for the main C++ repository. I'd be glad to be one of the maintainers.

@reyang Can you look on this PR once, as it's blocked on changes requested by you. Thanks :)

@reyang
Copy link
Member

reyang commented Feb 24, 2021

@reyang Can you look on this PR once, as it's blocked on changes requested by you. Thanks :)

I'll approve if we remove https://github.com/orgs/open-telemetry/teams/cpp-maintainers and https://github.com/orgs/open-telemetry/teams/cpp-approvers from the PR. We can either create separate groups (e.g. cpp-contrib-approvers) like what .NET did, or leave it as a TODO for next PR.

Remove notion of main repo contributors and maintainers
Clarify on CODEOWNER policy.
@maxgolov
Copy link
Contributor Author

maxgolov commented Mar 1, 2021

I'll approve if we remove https://github.com/orgs/open-telemetry/teams/cpp-maintainers and https://github.com/orgs/open-telemetry/teams/cpp-approvers from the PR.

@reyang - I removed the notion of the main repo maintainers and approvers. I added a reference to the CODEOWNERS process, which itself is TBD in a separate PR. Please have another look.

@maxgolov maxgolov dismissed reyang’s stale review March 1, 2021 22:14

Please take a look at updated text.

Copy link
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

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

LGTM.

@reyang reyang merged commit 73482c9 into main Mar 1, 2021
@reyang reyang deleted the maxgolov/dir_layout branch March 1, 2021 22:53
@marcalff marcalff added the general General issue, not specific to a particular contrib label May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general General issue, not specific to a particular contrib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create initial layout of directories
5 participants