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

Only check for public symbols in public packages/modules #2458

Merged
merged 1 commit into from
Feb 11, 2022

Conversation

aabmass
Copy link
Member

@aabmass aabmass commented Feb 10, 2022

Description

Update public symbols checker to skip files which:

  • Have a path part starting with a single leading underscore, for example opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py would not be checked but opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py would be

  • Have the full string tests as one of the path parts, for example opentelemetry-sdk/tests/metrics/test_metrics.py would not be checked but opentelemetry-sdk/src/opentelemetry/foo/equalitytests/foo.py would be.'

    Note this could skip a file like opentelemetry-sdk/src/opentelemetry/foo/tests/foo.py, but since some of our packages are within another directory e.g. exporter/opentelemetry-exporter-otlp-grpc I can't simply check the second path part.

Type of change

Tooling

How Has This Been Tested?

Tested by creating new commits on top that pass/fail the check, see #2459 CI run:

The code in this branch adds the following public symbols:

- opentelemetry-sdk/src/opentelemetry/sdk/trace/testsfoo.py
	Foo

- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
	Foo

Please make sure that all of them are strictly necessary, if not, please consider prefixing them with an underscore to make them private. After that, please label this PR with "Skip Public API check".

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

N/A

@aabmass aabmass requested a review from a team February 10, 2022 20:11
@aabmass aabmass added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Feb 10, 2022
@aabmass aabmass changed the title Only check for public symbols in public packages Only check for public symbols in public non-test packages/modules Feb 10, 2022
@aabmass aabmass changed the title Only check for public symbols in public non-test packages/modules Only check for public symbols in public packages/modules Feb 10, 2022
@aabmass aabmass added the build & infra Issues related to build & infrastructure. label Feb 10, 2022
Copy link
Contributor

@ocelotl ocelotl left a comment

Choose a reason for hiding this comment

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

Yes! a great enhancement 😎

@aabmass aabmass added the PR:please merge This PR is ready to be merged by a Maintainer (has enough valid approvals, successful build, etc.) label Feb 11, 2022
@lzchen lzchen merged commit a01c49f into open-telemetry:main Feb 11, 2022
@aabmass aabmass deleted the fix-public-checker branch February 11, 2022 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build & infra Issues related to build & infrastructure. PR:please merge This PR is ready to be merged by a Maintainer (has enough valid approvals, successful build, etc.) Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants