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

[stdlib] Mark UnicodeScalar.utf16 and UnicodeScalar.UTF16View as public #4929

Merged
merged 2 commits into from
Sep 26, 2016
Merged

[stdlib] Mark UnicodeScalar.utf16 and UnicodeScalar.UTF16View as public #4929

merged 2 commits into from
Sep 26, 2016

Conversation

eliperkins
Copy link

@eliperkins eliperkins commented Sep 22, 2016 •
edited
Loading

UnicodeScalar.utf16 and UnicodeScalar.UTF16View do not have access modifiers and are therefore internal. This marks them as public.

Resolves SR-2627.

@eliperkins
Copy link
Author

@swift-ci Please smoke test

@modocache
Copy link
Contributor

@swift-ci please smoke test

@gottesmm
Copy link
Contributor

@moiseev This is an API change, no? What is the policy around this?

@moiseev moiseev changed the title Mark UnicodeScalar.utf16 and UnicodeScalar.UTF16View as public [stdlib] Mark UnicodeScalar.utf16 and UnicodeScalar.UTF16View as public Sep 22, 2016
@moiseev
Copy link
Contributor

moiseev commented Sep 22, 2016

Thanks @gottesmm. This is indeed an API change and as such should go through the evolution. On the other hand, it can be considered a bug-fix.
@eliperkins can you initiate a swift-evolution discussion?

moiseev
moiseev reviewed Sep 22, 2016
View reviewed changes
stdlib/public/core/UnicodeScalar.swift
struct UTF16View {
var value: UnicodeScalar
public struct UTF16View {
public var value: UnicodeScalar
Copy link
Contributor

Choose a reason for hiding this comment

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

This property should remain internal, actually.

Copy link
Author

Choose a reason for hiding this comment

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

Addressed in 4a6d368

@eliperkins
Copy link
Author

can you initiate a swift-evolution discussion?

@moiseev Sure thing! Better to kick it off on the mailing list or just by opening up a proposal on swift-evolution?

@moiseev
Copy link
Contributor

moiseev commented Sep 22, 2016

Mailing list. If it ends up being considered a bug-fix, it might not even need a formal proposal.
Thanks!

@moiseev moiseev added the swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review label Sep 22, 2016
@eliperkins
Copy link
Author

Kicked off the email on swift-evolution. Thanks!

@eliperkins
Copy link
Author

@swift-ci please smoke test

@dabrahams
Copy link
Contributor

@swift-ci Please smoke test and merge

@moiseev moiseev removed the swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review label Sep 26, 2016
@moiseev
Copy link
Contributor

moiseev commented Sep 26, 2016

@swift-ci Please smoke test and merge

@swift-ci swift-ci merged commit 0ab20e8 into swiftlang:master Sep 26, 2016
@eliperkins eliperkins deleted the SR-2627-utf16-access-modifier branch September 26, 2016 15:34
@gribozavr
Copy link
Contributor

Could you add some tests?

@swift-ci swift-ci mentioned this pull request Sep 22, 2016
Closed
MaxDesiatov pushed a commit that referenced this pull request Sep 7, 2023
@kateinoigakukun
Merge pull request #4929 from swiftwasm/main
ad6f530 
[pull] swiftwasm from main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moiseev moiseev moiseev left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@eliperkins @modocache @gottesmm @moiseev @dabrahams @gribozavr @lattner @swift-ci