Refactor lib/middleware/ext/session.js

[mwbrooks]

This abstraction doesn't make sense. Let's rethink it..

[filmaj]

@mwbrooks care to dig deep into your May 27, 2014 memories to shed some light on what kind of refactor plans you had? :P

[filmaj]

Ping @surajpindoria @timkim in case y'all have more context as to what Mr. Brooks was thinking when he filed this....

[timkim]

This one is a little weird. It was originally used to figure out which platform/version of cordova a client needed. I think the re-think was figuring out a sane way of setting platforms and versions that were compatible. With content-sync, that part of session.js is no longer needed.

However, the module is still used to init some settings for each client. eg, if you look in https://github.com/phonegap/connect-phonegap/blob/master/lib/middleware/register.js#L5 you can see it setting some stuff.

[filmaj]

Based on the comments in that register module you linked to, it looks like it is only used to track cordova and device information within the session so that the proper cordova files are served. But, it sounds like content sync removes the need for this altogether.

Is this figure-out-which-cordova-to-serve the only thing we need to use sessions for? If so, perhaps we can remove the use of sessions altogether and simplify the module.

[timkim]

I think we still need the sessions object. It's used to store some more info used for the autoreload feature and content-sync. eg: https://github.com/phonegap/connect-phonegap/blob/master/lib/middleware/update.js#L44

However, the part where it figures out which cordova and what platform...probably won't need those.

[filmaj]

Right, makes sense.

May I suggest we close this issue down, as we don't really know how this ext/session module is to be refactored? Instead, all this stuff we've identified about session info we no longer need to track for serving particular cordova files, we drop that into #190 as part of what needs to be done for that issue?

[timkim]

SGTM.