-
Notifications
You must be signed in to change notification settings - Fork 759
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
Explain dependencies #6
Comments
Definitely. Could you help out with this? And I merge it in. I think what we need is:
|
Is the whole jQuery lib really needed here just for AJAX? I understand including Base64 so people can have a choice of basic auth or OAuth and you're using a few parts of Underscore to work with arrays better, so the 3k and 12k seem justified. These with the github.js make up around 28k. However, it then doubles in size by requiring jQuery minified lib and you seem to be just using it for AJAX. Could an AJAX lib or just the AJAX portion of jQuery not be used instead to keep things ~30k? |
Definitely. This was developed as part of prose.io and I guess jQuery dependency must have been brought over for easy ajax. If you try to shift, please try to use a cross-platform library, so that people could use it in node as well. |
Yeah. I'd love to get rid of the jQuery dependency, ideally in favor of a Raw XMLHttpRequest, which would make it compatible to node also. I just don't have time to fix it. Patches welcome. :) |
Started writing a raw XMLHttpRequest replacement which works well but needs finishing, probably tomorrow. Did have a couple of false starts though. Didn't know you had to register new applications in Github to avoid CORS errors from Github. Then tried repo.list but that doesn't seem to be in the lib? Great library when I got it working tho! :) |
Switched to directly referencing jquery, for better compatibility with other libraries
As of now, this needs jQuery, underscore, and an implementation of Base64.encode as well to function. Please note them down as such in the readme.
I think the Base64 should be included with it, at least.
The text was updated successfully, but these errors were encountered: