-
Notifications
You must be signed in to change notification settings - Fork 27.5k
Conversation
@sebastienroul - can you ensure that you have signed the CLA? Thanks. |
@petebacondarwin : Signed yesterday... Waiting for response :) |
Hi @sebastienroul , would you mind changing your commit messages and PR description to be consistent with our git commit guidelines? https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#git-commit-guidelines We use commit messages to auto-generate changelogs. It'd also be helpful if you could write a failing unit test (which passes with your implementation). |
Hi @jeffbcross : OK for the commit message : I've update it. Should I do this for all my commit messages or only for the first of the pull ?
In all thanks for your patience ! |
@sebastienroul you can squash these commits and correct your commit message according to the contributing guide with a rebase. This travis failure isn't a big deal, you can trigger a rebuild by re-pushing to github. As for writing tests, any change is going to need at least one test case to assist in preventing future regressions. I'm happy to assist in writing tests, or reviewing the tests you write, to ensure that they're as meaningful as possible, and prevent them from breaking as much as possible. Happy to help with this, but to start with you should write a test which verifies that your own particular use case is served. |
No worries ;)
|
Ahh yes, what @caitp said... a squash of commits would be helpful. |
@sebastienroul I'm reviewing this PR now, and I'll go ahead and squash the commits and change the message. |
Thanks for all ! continuous improvement :) |
@sebastienroul, I amended a test and some implementation changes to your commit, and re-opened it under this PR: #4928 |
fix(location,route): Breaks with file:// protocol in Chrome
FF and IE implementations considere the drive letter as "host" and remove it from pathname
CHROME implementation let it as part of the pathname
We should remove the drive letter from the pathname to let routing work.
Closes #4680