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 dismissal delegate method; remove documentation #1348

Merged
merged 2 commits into from
Apr 30, 2018

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Apr 28, 2018

Renamed NavigationViewControllerDelegate.navigationViewControllerDidEndNavigation(_:cancelled:) to navigationViewControllerDidDismiss(_:byCanceling:), which reads more like one of the standard UIKit view controller delegate methods. Also prefer the American spelling of cancel* over the British spelling.

Removed the delegate method section of the readme. That documentation was not only redundant to NavigationViewControllerDelegate’s documentation comments, but it also used inaccurate or outdated names.

/ref #1318
/cc @frederoni @vincethecoder

Also spelled “canceling” and “canceled” the American way, for consistency with the rest of the SDK’s public API.
This documentation is not only redundant to NavigationViewControllerDelegate’s documentation comments, but it also uses inaccurate or outdated names.
@1ec5 1ec5 added topic: documentation op-ex Refactoring, Tech Debt or any other operational excellence work. documentation labels Apr 28, 2018
@1ec5 1ec5 added this to the v0.17.0 milestone Apr 28, 2018
@1ec5 1ec5 self-assigned this Apr 28, 2018
Copy link
Contributor

@vincethecoder vincethecoder left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@vincethecoder vincethecoder left a comment

Choose a reason for hiding this comment

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

@1ec5 tests are currently failing btw.

@bsudekum bsudekum dismissed vincethecoder’s stale review April 30, 2018 18:20

@vincethecoder no need to request changes if tests are failing.

@1ec5 1ec5 merged commit 674454f into master Apr 30, 2018
@1ec5 1ec5 deleted the 1ec5-readme-dedelegate branch April 30, 2018 18:41
@1ec5 1ec5 changed the title Rename dismissal delegate method; removed documentation Rename dismissal delegate method; remove documentation May 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation op-ex Refactoring, Tech Debt or any other operational excellence work. topic: documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants