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

[wip] Universal Module Definition for require.js and friends, namespacing, bower, travis, etc. #18

Merged
merged 12 commits into from
Apr 4, 2014

Conversation

bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Apr 4, 2014

First off, love the work: I love seeing more stuff pushing the boundary of what browser-based visualization can do.

My original goal here was to get some stuff up and running against the new IPython 2.0 widgets: its system uses require.js, which just wasn't working out with the current build on the site/generated by previous scripts.

I have shimmed in the Univeral Module Definition which pretty much handles the IPython case, as well as whatever people would want to use: seemed like the most expedient way to get to something that works. This mainly means everything is now in the cola object that gets exported, so outside of the /src, it must be used for all the other things that used to be globally exposed by using cola.v1.min.js: powergraph, etc. Obviously a pretty big API change.

This lead to fixing up the tests, and in fiddling around with my IPython stuff, making a lean-ish bower definition.

I must confess, I haven't really looked at any of the code in a substantive way, so I am basically banking on the evidence of the tests and examples working again as for this evening's work being "good enough" for this to be a PR worth working towards inclusion, and not just some throw-away hacks.

Since I had spent time on the tests, I set up travis: https://travis-ci.org/bollwyvl/WebCola

Certainly interested in pursuing this further, and welcome any feedback!

tgdwyer added a commit that referenced this pull request Apr 4, 2014
[wip] Universal Module Definition for require.js and friends, namespacing, bower, travis, etc.

This is awesome!!
@tgdwyer tgdwyer merged commit 64485c0 into tgdwyer:master Apr 4, 2014
@tgdwyer
Copy link
Owner

tgdwyer commented Apr 4, 2014

Thank you bollwyvl! This is some really nice cleanup and I really appreciate you doing it. This is my first major javascript project and I'm learning as I go. Your contribution really helps to keep the quality high.

Do you mind if I give you a credit on the cola.js homepage: http://marvl.infotech.monash.edu/webcola/
?

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Apr 5, 2014

Sure, credit is fine with me. If this is your first JavaScript project, I'm sure we'll see some more impressive things soon!

Hopefully I'll have some other things to contribute, but have swung back to the problem at hand (rather than the problem's problems)! I was successful in getting cola added to the IPython notebook, but still have some more to do, and look forward to publishing that.

The approach I took for this go-around was pretty ham-fisted... really just wrapping your stuff. It's possible we may find a better structure for how to organize the build, and potentially make it more modular, but I'd have to dig some to figure out what's what.

What other goals do you have for the project in terms of community and automation stuff? I'll try some suggestions, and see if you like them :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants