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 …didDisplayPhoto… to …didNavigateToPhoto… #47

Merged
merged 1 commit into from
May 1, 2015

Conversation

cdzombak
Copy link
Contributor

This more closely reflects that this delegate method/notification is tightly tied to user interactions.

I'm taking the viewpoint here that #42 is a naming "bug", not a behavior bug as such, since the method's docs say:

Called when a new photo is displayed through a swipe gesture.

closes #42

This more closely reflects that this delegate method/notification is tightly tied to user interactions.

closes #42
@mbbischoff
Copy link
Contributor

Solid, but sadly a breaking change. :(

@cdzombak
Copy link
Contributor Author

sadly a breaking change.

yes, this would require a new major version. I considered deprecating the current API and adding the new one, which could only require a new minor version, but that seemed like too much complexity.

@mbbischoff
Copy link
Contributor

yes, this would require a new major version. I considered deprecating the current API and adding the new one, which could only require a new minor version, but that seemed like too much complexity.

Agreed. Totally agree that this should probably be merged.

@cdzombak
Copy link
Contributor Author

cdzombak commented May 1, 2015

@Twigz @steve-matthews thoughts?

@steve-matthews
Copy link
Contributor

I'm a 👍

Twigz added a commit that referenced this pull request May 1, 2015
Rename …didDisplayPhoto… to …didNavigateToPhoto…
@Twigz Twigz merged commit a95af4c into develop May 1, 2015
@cdzombak cdzombak deleted the cdz/42-delegate-method-naming branch May 26, 2015 14:14
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.

4 participants