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

Fernando/turbo navigator feedback #163

Merged
merged 3 commits into from
Nov 20, 2023

Conversation

olivaresf
Copy link
Member

[Re-opened after I removed the Turbo enum issue.]

Three minor things. Feel free to cherry-pick any commits to your branch or merge if all commits are good!

  • a code comment that I could've added to the original PR
  • protocol conformance to use PathConfigurationIdentifiable
  • renaming Navigation to TurboNavigation

Let me know what you think!

@@ -69,12 +69,15 @@ extension SceneController: UIWindowSceneDelegate {
extension SceneController: TurboNavigatorDelegate {
func handle(proposal: VisitProposal) -> ProposalResult {
switch proposal.viewController {
case "numbers":

Copy link
Member

Choose a reason for hiding this comment

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

Is there a style guide we can follow for Swift code? I feel like we've gone back and forth removing/adding these blank lines a few times!

Copy link
Member Author

Choose a reason for hiding this comment

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

😆

We don't use a style guide, but if you don't like the new lines, I'll adapt. I don't feel strongly about it.

Copy link
Member

Choose a reason for hiding this comment

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

To be honest, I just run SwiftFormat on every file before I save it!

@joemasilotti
Copy link
Member

LGTM! Feel free to merge.

@olivaresf olivaresf merged commit 1aae4fb into turbo-navigator Nov 20, 2023
2 checks passed
@olivaresf olivaresf deleted the fernando/turbo-navigator-feedback branch November 20, 2023 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants