search

rohitvarkey / ThreeJS.jl

Julia interface to WebGL using Three-js custom elements and Patchwork.jl
https://rohitvarkey.github.io/ThreeJS.jl
Other
56 stars 15 forks source link

Remove bower_components, but make installation of them automatic with Pkg.add #41

Closed izaid closed 7 years ago

izaid commented 7 years ago

The purpose of this PR is to make it easier to develop three-js with ThreeJS.jl. I think it's great that the three-js bower components are in their own repository, and they should remain so. But, it is currently quite annoying to have to make two separate PRs every time we change three-js. And it's only a matter of time until someone forgets to do that. Here is how I've tried to resolve this problem.

I've removed the bower_components from ThreeJS.jl. I've also added a deps/build.jl file that runs bower install rohitvarkey/three-js. This means that anyone installing ThreeJS.jl via Pkg.add("ThreeJS") will get the bower_components installed automatically. That script can also be run manually with Pkg.build("ThreeJS").

However, a Pkg.clone(...) will not get the bower_components by default. This will allow developers to clone the three-js git repository in the normal location in the assets directory, and develop them both at the same time.

I think this effectively solves the dual repository problem. I considered git submodules as well, but I don't think they fit here.

rohitvarkey commented 7 years ago

I agree that the duplication isn't the best way of working with three-js and is quite annoying.

However, I don't think we should we expecting users to have bower installed on their systems. Or require them to have bower to be able to use the package.

izaid commented 7 years ago

Hmmm, fair point. Thinking...

izaid commented 7 years ago

Are you okay with the basic idea here, that we have something in deps/build.jl that gets the bower_components? I'm pretty sure we can make something work then.

rohitvarkey commented 7 years ago

@izaid That is fine with me. But as I said before I don't want to force users to install bower or know how to use it. If we can figure something out that works well for users who want to use bower to mess around with the JS source, that'd be great. 😄

Also this approach wouldn't keep the bower_components directory updated with the latest updates to the JS right? Only the version that is installed will be on users computers, unless they explicitly run bower update. (Or does Pkg run Pkg.build everytime Pkg.update is called?)

codecov-io commented 7 years ago

Current coverage is 87.09% (diff: 100%)

Merging #41 into master will not change coverage

@@             master        #41   diff @@
==========================================
  Files             3          3          
  Lines            93         93          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits             81         81          
  Misses           12         12          
  Partials          0          0

Powered by Codecov. Last update 5357fe7...e3c8219

izaid commented 7 years ago

@rohitvarkey Okay, so with regard to bower, do we really need to install the JS using bower? Why don't we just simply download files. Or, at the very least, keep polymer and webcomponents in ThreeJS.jl (via bower), but grab three-js using wget or whatever? Escher, for instance, only stores the strict dependencies in its bower_components folder, not Escher's own HTML / JS.

As for the other point, I checked it out, and from what I can understand Pkg.update() calls Pkg.resolve() and Pkg.resolve() calls Pkg.build(). So that should be fine.

Conclusion: I think we should keep the strict dependencies (polymer and webcomponents) in the repository as is, but not install three-js via bower install. We instead install it dumbly by downloading or whatever via Pkg.build(). Thoughts?

rohitvarkey commented 7 years ago

It's all tradeoffs here!

If you decide to keep strict dependencies only in bower_components, we still need to make sure the versions in them match the ones three-js requires and update them when needed. These won't happen a lot, which I think makes it even likelier that developers will forget to update it. What do you think?

izaid commented 7 years ago

Agree that it's all tradeoffs. :)

I think the strict dependencies only in bower_components is better. To me, that's akin to having thirdparty libraries in the repository, which happens all the time. Developers may forget to update them, but it won't happen often, as you said. I see that as a plus. :)

I propose we try something like this, but move it back if we don't like it. Is that cool? If yes, I'll update the PR with the new plan.

izaid commented 7 years ago

Julia even has a download(...) command, so this should hopefully be easy. :)

rohitvarkey commented 7 years ago

That sounds okay to me. Can you write up some devdocs too and point at it from README.md so that new developers can follow along?

izaid commented 7 years ago

Absolutely. Okay, back in a bit with an update.

izaid commented 7 years ago

@rohitvarkey Agreed, I've updated it to the latest version. Also updated the README.

This should be good to go. At some point, I'll move three-js out of the bower_components directory and directly into assets. But I want to make these changes one step as a time.

I think this will be good. Simultaneous development of both repositories will help a lot.