search

justinfagnani / route

A client + server routing library for Dart
BSD 3-Clause "New" or "Revised" License
114 stars 40 forks source link

A lot of refactoring, some fixes #77

Closed vicb closed 9 years ago

vicb commented 10 years ago

I've kept separated commits so that the review is easier.

I've changed the code to make it easier to follow, it might be a matter of personal prefs though. If this is the case please pin-point the commit and the modif so that I can revert.

Tests are passing as well as before: there is one remaining issue with the last test in test_client.html.

pavelgj commented 10 years ago

FYI: master branch is quite old. If you're looking to contribute, see either:

https://github.com/dart-lang/route/tree/0.5.0

or

https://github.com/angular/route.dart

vicb commented 10 years ago

thanks @pavelgj could you please explain me the differences between dart-lang 0.5 and angular router please ?

Maybe something should be added to the README.md ? Github also allows setting the default target branch for PR...

pavelgj commented 10 years ago

Once there was master branch in dart-lang/route. It had the original simple implementation of routing and was published to pub as route. Then experimental_hierarchical branch came into existence with the fancy hierarchical router, which was later adopted by angular.dart for which purpose it was "forked" on pub as route_hierarchical. Somewhere in between, 0.5.0 branch came into existence with an effort to make hierarchical router cleaner, easier, better. Unfortunately, it wasn't progressing very fast due to lack of time. Meanwhile, as angular.dart was adding more users maintaining route_hierarchical in a branch was getting harder and harder and decision was made to fork it to https://github.com/angular/route.dart.

As it stands right now:

vicb commented 10 years ago

@pavelgj Thanks for your detailed explanation.

Do you think you can update the README.md with this text in the header ? that would be really helpful. Settings the default PR target branch to 0.5.0 in github might also help.

pavelgj commented 10 years ago

@sethladd Seth, how do you think we should handle the message?

sethladd commented 10 years ago

@pavelgj we could submit a PR with your explanation in the README. @justinfagnani any thoughts?

We could also merge 0.5.0 into master, which should help.