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

Make constructors of DatadogSite private #2010

Merged

Conversation

0xnm
Copy link
Contributor

@0xnm 0xnm commented Apr 25, 2024

What does this PR do?

Constructors of DatadogSite shouldn't be public.

This may be see as a breaking change (removing items from public API), but this was never intended to be public.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@0xnm 0xnm requested review from a team as code owners April 25, 2024 10:33
@codecov-commenter
Copy link

codecov-commenter commented Apr 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.77%. Comparing base (cff8b54) to head (9354705).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2010      +/-   ##
===========================================
+ Coverage    83.71%   83.77%   +0.06%     
===========================================
  Files          488      488              
  Lines        17736    17767      +31     
  Branches      2659     2666       +7     
===========================================
+ Hits         14846    14883      +37     
+ Misses        2177     2169       -8     
- Partials       713      715       +2     
Files Coverage Δ
...src/main/kotlin/com/datadog/android/DatadogSite.kt 100.00% <100.00%> (ø)

... and 30 files with indirect coverage changes

Copy link
Collaborator

@xgouchet xgouchet left a comment

Choose a reason for hiding this comment

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

That works, but it isn't an issue, as enum have a private init.
If someone tried to write the following, it wouldn't compile anyway :

val customSite = DatadogSite("custom", "custom-intake.example.com")

This line triggers two compilation errors:

  • Cannot access '<init>': it is private in 'DatadogSite'
  • Enum types cannot be instantiated

@0xnm
Copy link
Contributor Author

0xnm commented Apr 26, 2024

@xgouchet yes, my concern was mostly the look of the API surface file, these custom constructors are declared there.

@0xnm 0xnm merged commit a7e928a into develop Apr 26, 2024
21 checks passed
@0xnm 0xnm deleted the nogorodnikov/make-constructors-of-datadog-site-private branch April 26, 2024 07:41
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.

None yet

4 participants