Luke and I had a discussion on IRC about this. Basically, the more I think about it, the
more I think most of what is happening in AeroGear.ajax is either already handled by
jQuery.ajax or can be factored out into a smaller helper or merged into the specific part
of the library it was meant to help. This will have the advantages of simplifying the lib
to make it easier to grok, allow the decoupling of AeroGear.Auth and AeroGear.Pipeline
(see
https://issues.jboss.org/browse/AEROGEAR-858) and also could shrink the size of the
library a bit as well.
Below is the IRC transcript from the conversation Luke and I had. Anyone have any other
opinions?
[09:28:37] <+kborchers> lholmquist: need your opinion
[09:28:52] <+lholmquist> kborchers: yes, i agree
[09:28:56] <+lholmquist> oh wait
[09:28:59] <+kborchers> :)
[09:29:13] <+kborchers> lholmquist: how much have you looked at / understand what is
going on in AeroGear.ajax?
[09:29:29] <+kborchers> lholmquist: the reason i ask is i am questioning its
existence
[09:29:59] <+lholmquist> kborchers: a bit, i would need to look at it to refresh
my memory
[09:30:28] jdoyle (~jdoyle(a)pool-96-233-74-235.bstnma.fios.verizon.net) joined the
channel.
[09:30:41] <+lholmquist> kborchers: which pieces are you thinking should go?
[09:30:47] <+kborchers> lholmquist: i was thinking about it last night and now that
i read through it, the more i look at it the more i find myself saying … "Doesn't
jQuery already do that for us?"
[09:32:10] <+kborchers> there are only 2 pieces i'm not sure about
[09:32:12] <+kborchers> 1
[09:32:25] <+kborchers> the POST/PUT of JSON formatted data being stringified
[09:32:26] <+lholmquist> there is the Auth stuff in there
[09:32:27] <+kborchers> 2
[09:32:42] <+kborchers> the auth check to prevent unnecessary http requests
[09:32:52] <+kborchers> #2 i know is not done by jQuery.ajax but
[09:33:13] <+lholmquist> could that be done somewhere else then?
[09:33:18] <+kborchers> i think that could be moved to auth, and auth refactored to
decouple it from pipeline (which i want to do anyway)
[09:33:45] <+kborchers> and i think the post put stuff could just happen in
pipeline
[09:34:39] <+kborchers> i think i'm going to play with this today and see if i
can kill AeroGear.ajax
[09:34:41] <+lholmquist> all the promise stuff is handled by jquery anyway
isn't it
[09:35:18] <+kborchers> lholmquist: yeah, except for rejecting it immediately based
on auth on line 58 of aerogear.utilities.js
[09:35:53] abstractj is now known as abstractj|away
[09:36:05] <+kborchers> but like i said
[09:36:10] <+kborchers> i think that can move into Auth
[09:36:19] <+kborchers> i think this is doable
[09:36:47] <+kborchers> lholmquist: any concerns that this may be a bad way to go
right now?
[09:36:57] <+kborchers> removing AeroGear.ajax i mean
[09:37:16] <+kborchers> it should cut or file size a bit too :)
[09:37:20] <+kborchers> s/or/our
[09:38:29] <+lholmquist> kborchers: i don't think so, the majority of what it
does is parse the returned data to a proper format it looks like
[09:39:30] <+kborchers> which can be handled in the callbacks
[09:39:44] <+kborchers> ok
[09:39:58] <+lholmquist> that is true, but is that an extra step for the dev?
[09:40:19] <+kborchers> nope, because i have my own callback which runs first and
then calls theirs :)
[09:41:05] csadilek (csadilek@nat/redhat/x-mrudioyuhbqnjezq) joined the channel.
[09:41:12] <+kborchers> lholmquist:
https://github.com/aerogear/aerogear-js/blob/master/src/pipeline/adapters...
[09:41:28] <+kborchers> then i apply() their supplied callback
https://github.com/aerogear/aerogear-js/blob/master/src/pipeline/adapters...
[09:41:34] <+qmx> kborchers: you mean after the release right?
[09:41:39] <+kborchers> qmx: oh yeah
[09:41:42] <+kborchers> not for this one
[09:41:59] <+kborchers> we've already tagged
[09:42:01] <+kborchers> qmx: ^
[09:42:15] <+qmx> kborchers: yeah, just remembered that
[09:42:19] qmx brainfarted
[09:42:21] <+kborchers> :)
[09:42:26] <+lholmquist> kborchers: i don't see where the data gets parsed
here,
[09:42:46] jamezp is now known as jamezp_afk
[09:42:49] <+kborchers> lholmquist: it doesn't because it happens in
AeroGear.ajax right now … it would move in there though
[09:43:36] <+lholmquist> kborchers: ah, thats what i was missing. Then
wouldn't we have to repeat code for the other methods?
[09:43:48] mbg (~marius@redhat/jboss/mbg) left IRC. (Quit: mbg)
[09:44:35] <+kborchers> lholmquist: we could factor that out into its own utility
instead of bundling it into the ajax one
[09:44:42] <+kborchers> then it could be reused
[09:44:45] <+sblanc> kborchers: lholmquist: If you write this all down in to a JIRA
I will be happy to give a third opinion :)
[09:45:41] <+lholmquist> kborchers: that could work, in fact, would probably be
better
[09:46:01] <+kborchers> sblanc: sure, i will create a jira. are you saying you have
another opinion or you want some time to think on it to comment?
[09:46:26] <+sblanc> kborchers: Want some time to think about it :)
[09:48:15] <+lholmquist> kborchers: atm, the auth methods, login, enroll,
don't do any data parsing since they don't go through Aerogear.ajax, should we add
this new utility method to that once it's written?
[09:50:15] <+kborchers> lholmquist: maybe … probably … not sure yet. :) i think the
main reason i didn't use AeroGear.ajax was because it did an auth check and that
wouldn't be good during auth :)