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

Rename EntryPoint's _from_text() to from_text() and document it - [closed] #242

Closed
jaraco opened this issue Oct 22, 2020 · 6 comments
Closed

Comments

@jaraco
Copy link
Member

jaraco commented Oct 22, 2020

In GitLab by @jwodder on Mar 26, 2020, 17:39

Merges expose-ep-read-text -> master

Closes #114

@jaraco
Copy link
Member Author

jaraco commented Oct 22, 2020

In GitLab by @codecov on Mar 26, 2020, 17:44

Codecov Report

Merging #117 into master will not change coverage by %.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #117   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          260       260           
  Branches        21        21           
=========================================
  Hits           260       260           
Impacted Files Coverage Δ
importlib_metadata/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 654dc66...584ef41. Read the comment docs.

@jaraco
Copy link
Member Author

jaraco commented Oct 22, 2020

In GitLab by @jwodder on Mar 26, 2020, 17:45

@jaraco I realize that you gave me an alternative way to accomplish what I want without the need for this PR. You don't have to merge this; I'd just prefer it if you did.

@jaraco
Copy link
Member Author

jaraco commented Oct 22, 2020

In GitLab by @jaraco on Mar 27, 2020, 24:39

My main reservation with this change is that it makes (or strengthens) the format of the entry points (as) a public interface. Clearly, it was intended to remain a private interface, allowing for more flexibility for changing the underlying format. What's the motivation for accepting this change and burning that bridge?

@jaraco
Copy link
Member Author

jaraco commented Oct 22, 2020

In GitLab by @jwodder on Mar 27, 2020, 16:45

So you're saying _from_text shouldn't be public in case one day there's also (say) a _from_json and you want the decision for choosing which to call to be none of the end-users' business? OK, then; I defer to your vision for the project.

@jaraco
Copy link
Member Author

jaraco commented Oct 22, 2020

In GitLab by @jwodder on Mar 27, 2020, 16:45

closed

@jaraco
Copy link
Member Author

jaraco commented Oct 22, 2020

In GitLab by @jaraco on Mar 28, 2020, 24:29

So you're saying _from_text shouldn't be public?

Yes, essentially. I don't feel strongly about it, but I am cautious about expanding the advertised API surface without a compelling use-case.

Still, I welcome you to use _from_text, knowing it's a private interface and subject to change, if that suits your needs. Or if you feel having parsing support is really valuable and belongs here, just write up a justification. Thanks for the good spirit.

@jaraco jaraco closed this as completed Oct 22, 2020
jaraco added a commit that referenced this issue Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant