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.

Sign up for GitHub

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

Jump to bottom

Reduce visibility AiCoreService#DOTENV from public to private #136

Merged
merged 2 commits into from
Nov 5, 2024
Merged

Reduce visibility AiCoreService#DOTENV from public to private #136

merged 2 commits into from
Nov 5, 2024

Conversation

newtork
Copy link
Contributor

@newtork newtork commented Nov 4, 2024 •
edited
Loading

Exposing this field publicly leads to some risks:

  • User expects API stability
  • .. for a field with a third-party class
  • Implicit invitation for more util fields (feature creep)
  • Auto-completion may confuse the user

@newtork newtork added the please-review Request to review a pull-request label Nov 4, 2024
CharlesDuboisSAP
CharlesDuboisSAP reviewed Nov 4, 2024
View reviewed changes
Copy link
Contributor

@CharlesDuboisSAP CharlesDuboisSAP left a comment

Choose a reason for hiding this comment

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

my bad

core/src/main/java/com/sap/ai/sdk/core/AiCoreService.java Outdated Show resolved Hide resolved
@newtork @CharlesDuboisSAP
Update core/src/main/java/com/sap/ai/sdk/core/AiCoreService.java
7952a04 
Co-authored-by: Charles Dubois <103174266+CharlesDuboisSAP@users.noreply.github.com>
@newtork newtork changed the title Reduce visibility AiCoreService#DOTENV from public to protected Reduce visibility AiCoreService#DOTENV from public to private Nov 4, 2024
@newtork newtork merged commit c97527a into main Nov 5, 2024
5 checks passed
@newtork newtork deleted the reduce-visibility branch November 5, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CharlesDuboisSAP CharlesDuboisSAP CharlesDuboisSAP approved these changes

Assignees
No one assigned
Labels
please-review Request to review a pull-request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@newtork @CharlesDuboisSAP