Refactor 2 - WIP #20
Refactor 2 - WIP #20
Conversation
brian-c
force-pushed
the
epic-refactor
branch
from
b95f074 to
7e70b9f
Compare
|
Tests are all passing in Node now, wooo. Some changes to json-api-client were required, which I'm making in the fix-requests branch there. I took some liberty with the non-Node-module bits: the dist/...js file is no longer included, so if someone wants a single file, they need to run the build script themselves (we should really really discourage this). And I deleted the bower file: bower is doomed, nobody should be starting a new project with it. Any objections? Still need to try plugging it into the PFE to sure it still works in there, but the API is the same now so I think it'll be cool. |
|
Awesome :) Nope. Bower's done, and if people want to use it in a new project they can install this via Git. Same goes for the I want to add one more feature, which is OAuth signin (a la Wildcam Gorongosa etc). I might try doing that in a separate branch and merging it here though |
|
I've also added |
|
|
||
| var sugarClient = new SugarClient(); | ||
|
|
||
| auth.listen('change', function() { |
There was a problem hiding this comment.
@brian-c auth is undefined here - should be authClient? And I don't think change is emitted anymore..?
There was a problem hiding this comment.
Switched all those refs to auth, I don't really consider that module a client.
The module should still emit change events, is it not doing that somewhere?
There was a problem hiding this comment.
Fair enough. Still can't find a change event being emitted in there though...
(Happy New Year btw :) )
|
Awaiting a version bump after zooniverse/json-api-client#24, then I think this is solid. Major version bump. |
|
All set now. |
brian-c
force-pushed
the
epic-refactor
branch
from
c22f29e to
de8bfee
Compare
|
@brian-c Looks good. The only thing that I can see is that issue in |
|
Just thought of another thing. Given that |
|
@rogerhutchings Change events are fired whenever |
|
Ah - I think I was just being dumb and looking for the event to be emitted explicitly. I think this is good to go! |
|
Fixed. |
Tests passing for me and looks good. 💯
So, following standup today, we've decided to:
apifolderThis makes Panoptes-Front-End the standard, allows us to replace the API code in there, and means we have a working client for custom projects (including AnnoTate and Shakespeare's World, which already use the lib).
This will also supersede #14.