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

[4.x] Avoid exposing internal functions and properties #8137

Merged
merged 2 commits into from
Dec 24, 2023

Conversation

Goooler
Copy link
Contributor

@Goooler Goooler commented Dec 12, 2023

This is a backport for 4.x branch from #7734.

@Goooler
Copy link
Contributor Author

Goooler commented Dec 12, 2023

Something like List<T>.toImmutableList() should not be used by consumers

@Goooler Goooler force-pushed the internal-extension-backport branch from 76a13d3 to 1fd8849 Compare December 12, 2023 15:56
This is a backport for 4.x branch from f408411.
@Goooler Goooler force-pushed the internal-extension-backport branch from 45438ef to bc7ebb5 Compare December 13, 2023 01:53
@Goooler Goooler changed the title Avoid exposing internal functions and properties [4.x] Avoid exposing internal functions and properties Dec 13, 2023
@yschimke yschimke requested a review from swankjesse December 23, 2023 12:21
@yschimke
Copy link
Collaborator

@Goooler thanks for the fix. Not sure when it will be out, but it's a good idea to backport.

@yschimke yschimke merged commit 710bdc1 into square:okhttp_4x Dec 24, 2023
14 checks passed
@Goooler Goooler deleted the internal-extension-backport branch December 24, 2023 10:49
@swankjesse
Copy link
Collaborator

This is good, but why backport an internal refactoring to 4.x? This doesn’t give us any maintainability benefits but it does make accepting a 4.x upgrade more difficult?

swankjesse pushed a commit that referenced this pull request Apr 1, 2024
@swankjesse
Copy link
Collaborator

#8319

@Goooler
Copy link
Contributor Author

Goooler commented Apr 1, 2024

As I see something like List<T>.toImmutableList() exposed to users, it's easy to get confused and import okhttp internal APIs instead of others.

@swankjesse
Copy link
Collaborator

That kind of issue is super annoying, and I’m grateful that Kotlin introduced the internal modifier. But unfortunately I think a bunch of people accidentally use those APIs, and I don’t want to break their ability to upgrade OkHttp until they go to 5.x.

@Goooler
Copy link
Contributor Author

Goooler commented Apr 1, 2024

Makes sense!

swankjesse pushed a commit that referenced this pull request Apr 1, 2024
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.

3 participants