witoldsz / angular-http-auth

MIT License
2.37k stars 416 forks source link

Service Renaming #53

Open felipeleusin opened 10 years ago

felipeleusin commented 10 years ago

Hi,

Thanks for the code, this was the most well done version of this interceptor I found. Just wanted to suggest one small change: How do you feel changing authService to something more unique? It's a too generic name for a third party service and prevent's me to creating my own authService. Have you considered naming it httpAuthService or something?

christianrondeau commented 10 years ago

I agree, I had the same issue.

toymachiner62 commented 10 years ago

+1

christianrondeau commented 10 years ago

I made available a pull request (#70) which solves the problem by completely removing the dependency to authService.

dnauck commented 10 years ago

+1

ralphschindler commented 9 years ago

:+1:

tamlyn commented 9 years ago

+1

vlapo commented 9 years ago

+1

witoldsz commented 9 years ago

The problem is that it will brake all the apps using this module. I do this change, bump a major version number, but NPM or Bower will pick what's new and all the apps will crash.

toymachiner62 commented 9 years ago

@witoldsz only if people are using "" in their package.json/bower.json file. If that's the case, it's the users own fault for using "" in their .json file as you should always specify a major version using either carot or tilde http://stackoverflow.com/questions/22343224/difference-between-tilde-and-caret-in-package-json

The point of a major version change is to account for non-backwards compatible or breaking changes.

vlapo commented 9 years ago

I agree @toymachiner62. I read some issues and PR and I think there is (or will be in short time) space to release new major version with breaking changes (and good documentation).

So, maybe create new branch first and plan changes :)

I think part of new version could also be some dist folder with .min.js version and grunt/gulp to create it.

witoldsz commented 9 years ago

I think part of new version could also be some dist folder with .min.js version and grunt/gulp to create it.

Wouldn't it be strange if the source code repository had also the dist files? Committed on each source change? What about strange merge conflicts? Doesn't seem right, does it?

vlapo commented 9 years ago

Dont think so. Dist folder will be update only on new release. I see it in a lot of simple angular/JS libraries. E.g. https://github.com/eriktufvesson/ngBootbox.

However angular.js make it with separate bower repository. See https://github.com/angular/bower-angular.git

I think this is only on owner (and contributors) how to do it :) But I think minimized version could be useful.

witoldsz commented 9 years ago

Update on each release seems like a good compromise.

vlapo commented 9 years ago

Yeah. But contributors cannot commit to dist folder in PR, dont forget build on release, change bower conf, create some minify task and so on.

I know little bit painful to reach small goal as minimized version, its your call ;)