Skip to content

Conversation

@legendecas
Copy link
Member

Which problem is this PR solving?

Short description of the changes

Given that applying the private access modifier could be a potential breaking change, this does not apply the rule to every package to reduce the burden of reviewers.

  • Apply rule @typescript-eslint/explicit-member-accessibility to (non-instrumentation) packages (less changes).

@github-actions
Copy link
Contributor

This package does not have an assigned component owner and is considered unmaintained. As such this package is in feature-freeze and this PR will be closed with 14 days unless a new owner or a sponsor (a member of @open-telemetry/javascript-approvers) for the feature is found. It is the responsibility of the author to find a sponsor for this feature.
Are you familiar with this package? Consider becoming a component owner.

private readonly DEFAULT_CGROUP_V2_PATH = '/proc/self/mountinfo';
private readonly UTF8_UNICODE = 'utf8';
private readonly HOSTNAME = 'hostname';
private readonly MARKING_PREFIX = ['containers', 'overlay-containers'];
Copy link
Member

Choose a reason for hiding this comment

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

Nice cleanup, looks like those constants aren't used here and are already being exported in utils.

Copy link
Member

@JamieDanielson JamieDanielson left a comment

Choose a reason for hiding this comment

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

Should this be marked as breaking since there are some private modifiers being added in where they weren't previously?

Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

Looks good overall, but I think we may have to undo the changes from public to private in packages/resource-detector-aws, as the package is already stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.