Skip to content
This repository was archived by the owner on Feb 28, 2022. It is now read-only.

Removed $q dep, opts passing, optimized promises #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nyurik
Copy link

@nyurik nyurik commented Oct 7, 2017

  • in api, this.options was not passed to osmAPI
  • removed the need for $q - native promises are better
  • in many cases, there was a wrapping for promises

Please check if the PR fulfills these requirements

  • The commit(s) message(s) follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What is the current behavior? (You can also link to an open issue here)

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

* in api, this.options was not passed to osmAPI
* removed the need for $q - native promises are better
* in many cases, there was a wrapping for promises
@nyurik
Copy link
Author

nyurik commented Oct 7, 2017

Apparently PhantomJS is the only component out there that doesn't support Promises... In any case, I think I will fork out the API component, as it is highly useful as a standalone feature outside of Angular. I think https://github.com/duivvv/generator-module-boilerplate would be a good starting point for it.

@jmfrancois
Copy link
Collaborator

I was about to start something like that for React, so creating kind of OSM API just in javascripts.

This lib was one of the first I have created (jmfrancois === toutpt)
Now I would just remove webpack config and just use babel to build a lib folder

@nyurik
Copy link
Author

nyurik commented Oct 9, 2017

@jmfrancois ideally, we should have a simple lib that can be used from the browser AND nodejs (which means XHR needs to be plug-able). The angular-osm would use this lib, avoiding any code dups, and have a good starting point for anyone. Thx!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants