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

Rename StyleManagerDelegate.locationFor(styleManager:) #1577

Closed
1ec5 opened this issue Jul 24, 2018 · 2 comments
Closed

Rename StyleManagerDelegate.locationFor(styleManager:) #1577

1ec5 opened this issue Jul 24, 2018 · 2 comments
Labels
op-ex Refactoring, Tech Debt or any other operational excellence work.

Comments

@1ec5
Copy link
Contributor

1ec5 commented Jul 24, 2018 •
edited
Loading

StyleManagerDelegate.locationFor(styleManager:) should be renamed StyleManagerDelegate.location(for:) to conform to Swift API design guidelines. (At the same time, it should have an Objective-C name of -locationForStyleManager:.)

/cc @JThramer
@1ec5 1ec5 added op-ex Refactoring, Tech Debt or any other operational excellence work. quick fix labels Jul 24, 2018
@1ec5
Copy link
Contributor Author

1ec5 commented Jul 24, 2018

Better yet, let’s say why the style manager wants a location, because it isn’t obvious: styleManager(_:horologicalLocation:) or somesuch.

@bsudekum
Copy link
Contributor

It might be interesting as well to remove this delegate method completely. What if we just listened in on progress did change notifications?

@frederoni frederoni mentioned this issue Sep 18, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
op-ex Refactoring, Tech Debt or any other operational excellence work.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bsudekum @1ec5