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

Jump to bottom

x/exp/utf8string: rename NewString to New #28415

Closed
ainar-g opened this issue Oct 26, 2018 · 4 comments
Closed

x/exp/utf8string: rename NewString to New #28415

ainar-g opened this issue Oct 26, 2018 · 4 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Unreleased

Comments

@ainar-g
Copy link
Contributor

ainar-g commented Oct 26, 2018

I am not sure what's the policy on x/exp API stability, so feel free to close or make into a proposal if needed.

There are container/list.New, text/template.New, log.New, etc. I think that utf8string.NewString shouldn't stutter either and should become utf8string.New.
@gopherbot gopherbot added this to the Unreleased milestone Oct 26, 2018
@agnivade
Copy link
Contributor

/cc @robpike @adg

I did not see any owners in the doc. Figured you guys would be able to answer from the commit history.

@agnivade agnivade added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 31, 2018
@mpvl
Copy link
Contributor

mpvl commented Oct 31, 2018

This is a leftover from when this functionality was in core, pre 1.0. In general, I would not break the API gratuitously, even when in exp. If anything, New could be added and NewString marked as deprecated.

I'm interested to know what kind of use cases people have for this package, btw.

@ainar-g
Copy link
Contributor Author

ainar-g commented Nov 7, 2018

I don't have a use case for it to be honest. I just stumbled upon it while looking at stuff in x/exp.

If the response is "Don't touch it, it's basically frozen", I guess this issue can be closed, although a notice in the package doc saying

This is a leftover from when this functionality was in core, pre 1.0. Please don't use.

would be nice.

@mpvl
Copy link
Contributor

mpvl commented Nov 8, 2018

It is fine to use this package, but my suspicion was it would not be used in practice much. So I'm just always curious about actual use cases. But it is okay to leave it as is.

@mpvl mpvl closed this as completed Nov 8, 2018
@golang golang locked and limited conversation to collaborators Nov 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@agnivade @ainar-g @mpvl @gopherbot