Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replaced jStorage with store #6994

Merged
merged 4 commits into from Nov 9, 2018

Conversation

Projects
None yet
4 participants
@Nerdinacan
Copy link
Contributor

commented Nov 9, 2018

jStorage is giving me some grief in another branch.

Ostensibly It's just a local storage library... but it was written as a plugin for jQuery, which is kind of like hiring a tax attorney for your cat. It's also poorly-written and problematic to import with webpack due to basic incompetence issues and frequent references to window.$ instead of a passed jQuery parameter in a module wrapper.

Oh yeah! We also, only use it in about 1 place, so we get all the hassle with virtually no payoff.

(Ok I'm done venting.)

Store is a standard npm multi-storage wrapper conforming to the normal storage javascript API, works with local/session/whatever. It's basic. Like all xStorage wrappers it's barely necessary, but still offers backwards compatibility for older browsers while conforming to a modern javascript API.

Its biggest selling point for me is that it's not bolted Frankenstein-style to the side of jQuery.

I'm open to different libs if somebody has strong yearnings, so long as it isn't tied to jQuery or Backbone or some other travesty, and more or less conforms to the standard Web Storage API

@galaxybot galaxybot added the triage label Nov 9, 2018

@galaxybot galaxybot added this to the 19.01 milestone Nov 9, 2018

@Nerdinacan

This comment has been minimized.

Copy link
Contributor Author

commented Nov 9, 2018

Anybody know how many chickens I have to sacrifice to get that last Travis job to pass? It's passing with this exact build over on my Travis instance, and locally, but chokes here. Seems strange that my little client-side change would affect a server-side unit test
.
https://travis-ci.org/Nerdinacan/galaxy/builds/452701586

Nerdinacan added some commits Nov 9, 2018

Update jquery.custom.js
Committing just to make the tests run again.
@martenson

This comment has been minimized.

Copy link
Member

commented Nov 9, 2018

@Nerdinacan it seems that was the right number of chickens. Save the rest.

@martenson martenson merged commit fc87b19 into galaxyproject:dev Nov 9, 2018

6 checks passed

api test Build finished. 439 tests run, 1 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 190 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 268 tests run, 10 skipped, 0 failed.
Details
selenium test Build finished. 148 tests run, 2 skipped, 0 failed.
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details
@dannon

This comment has been minimized.

Copy link
Member

commented Nov 9, 2018

To follow up here, we're actually removing this completely and using localstorage/etc directly as we do in the rest of the app. Since this has already been merged, we'll follow up in subsequent PRs to finish it.

@Nerdinacan Nerdinacan deleted the Nerdinacan:replace_jstorage branch Nov 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.