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

Compass 3.0.0 #33

Merged
merged 19 commits into from
Aug 23, 2016
Merged

Compass 3.0.0 #33

merged 19 commits into from
Aug 23, 2016

Conversation

vadymmarkov
Copy link
Contributor

@vadymmarkov vadymmarkov commented Aug 23, 2016

@zenangst @RamonGilabert @onmyway133
We kinda like to break Compass from time to time, so it's a brand new breaking change 😄

  1. Remove Compass.parse completion closure in favour of Location struct:

So instead of:

func application(app: UIApplication,
  openURL url: NSURL,
  options: [String : AnyObject]) -> Bool {
    return Compass.parse(url) { route, arguments in
      switch route {
        case "profile:{username}":
          let profileController = profileController(title: arguments["{username}"])
          self.navigationController?.pushViewController(profileController, animated: true)
        default: break
      }
    }
}

We unwrap optional Location:

func application(app: UIApplication,
  openURL url: NSURL,
  options: [String : AnyObject]) -> Bool {
    guard let location = Compass.parse(url) else {
      return false
    }

    let arguments = location.arguments

    switch location.path {
      case "profile:{username}":
        let profileController = profileController(title: arguments["{username}"])
        self.navigationController?.pushViewController(profileController, animated: true)
      default: break
    }

    return true
}

The advantage of this approach is that Compass.parse is not async anymore, we get the needed result straight away and can handle the case when urn could not be parsed.

  1. Refactor Router and Routable to support Mac by using type aliases.
  2. Route path, arguments and fragments are scoped by Location struct. Makes it easier to add more properties in the future.
  3. We used only 2 String methods from Sugar and I feel like it's unnecessary to hold this dependency, especially thinking about Swift 3 where it's no longer actual. So... we don't use Sugar in Compass anymore 😄
  4. Refactor project structure to be prepared to Swift Package Manager.
  5. Add Swiftlint, mention-bot and update Travis config.

@mention-bot
Copy link

@vadymmarkov, thanks for your PR! By analyzing the annotation information on this pull request, we identified @zenangst, @jessearmand and @onmyway133 to be potential reviewers

@jessearmand
Copy link
Contributor

I agree with those ideas. I have removed Sugar as a dependency as well in my own fork.

s.ios.source_files = 'Sources/{iOS,Shared}/**/*'
s.osx.source_files = 'Sources/{Mac,Shared}/**/*'
s.dependency 'Sugar'
s.ios.source_files = 'Sources/**/*'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can reduce to just s.source_files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@vadymmarkov vadymmarkov changed the title Compass 4.0 Compass 3.0.0 Aug 23, 2016
@vadymmarkov
Copy link
Contributor Author

Well, seems like it's gonna be v3 because the last released is v2. We just had a wrong one in Podspec.

@onmyway133 onmyway133 merged commit b061d66 into master Aug 23, 2016
@onmyway133 onmyway133 deleted the feature/parse-result branch August 23, 2016 18:58
@onmyway133
Copy link
Contributor

🎸 🎶 🎶

@zenangst
Copy link
Contributor

This is grrreeeeat!

young-shaq-dancing-shaq-gifs

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.

5 participants