8000 remove js components from the repo by minrk · Pull Request #3240 · ipython/ipython · GitHub
[go: up one dir, main page]
Skip to content

remove js components from the repo #3240

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
May 2, 2013
Merged

remove js components from the repo #3240

merged 10 commits into from
May 2, 2013

Conversation

minrk
Copy link
Member
@minrk minrk commented Apr 29, 2013

Unfortunately, we can't rely entirely on package systems like bower,
since we want IPython to be usable from git without requiring users to have node.

This PR just moves the third-party components to a separate repo (ipython/ipython-components), and uses this as a submodule. That way we don't pollute the real repo tracking all kinds of irrelevant files. It does mean that a fresh checkout of IPython must run

git submodule init
git submodule update

to get started. With this, updating to a new version of CodeMirror, etc. should be easier to review, because the diff related to changing CodeMirror will just be a changed hash, rather than hundreds of changed files.

@bfroehle
Copy link
Contributor

This is methodology is fine by me. We'll need to update our documentation with the new git commands. This won't break our tarball build script, will it?

Lastly, can we detect the case where the user is running from git but does not have the component submodule checked out? If so, it'd be great to provide the submodule commands right then and there.

@ahmadia
Copy link
Contributor
ahmadia commented Apr 29, 2013

If you use: git clone --recursive repo

git will automatically clone and checkout all submodules (and submodules of
submodules) for you in the initial clone, simplifying things a bit.

@minrk
Copy link
Member Author
minrk commented Apr 30, 2013

Lastly, can we detect the case where the user is running from git but does not have the component submodule checked out?

Yes, this should be easy enough to add as part of the setup.py process

@bfroehle
Copy link
Contributor

Lastly, can we detect the case where the user is running from git but does not have the component submodule checked out?

Yes, this should be easy enough to add as part of the setup.py process

And in the python ipython.py notebook process?

@minrk
Copy link
Member Author
minrk commented Apr 30, 2013

And in the python ipython.py notebook process?

I guess we could, though I don't find that to be a particularly important entry point.

@takluyver
Copy link
Member

What will those of us with existing checkouts need to do?

@minrk
Copy link
Member Author
minrk commented Apr 30, 2013 
git submodule init
git submodule update

@juliantaylor
Copy link
Contributor

fyi, this will break the Ubuntu daily build until all dependencies are packaged.
The launchpad importer can't deal with submodules.

@minrk
Copy link
Member Author
minrk commented Apr 30, 2013

Thanks for the heads up. What would be involved in working around launchpad's failure?

@juliantaylor
Copy link
Contributor

One could have a second repository which merges the two official repos into one regular one and launchpad imports that instead.

For the official package I need to get all dependencies packaged and build using those instead of embedded copies anyway, thats probably the better solution.

minrk added 2 commits April 30, 2013 15:07
@minrk
fix submodule
8649b1c
@minrk
ensure submodules exist prior to doing anything
825bedf 
git submodule update fires before distutils gets fired up
because distutils cannot be trusted.
@minrk
Copy link
Member Author
minrk commented Apr 30, 2013

setup.py install or setupegg.py develop should work fine now from a fresh checkout, as does

pip install -e git://github.com/minrk/ipython@submodules#egg=ipython

Even python ipython.py makes sure submodules are checked out.

@ellisonbg
Copy link
Member

Main fabfile.py should be updated in the top level notebook dir.

@minrk
Copy link
Member Author
minrk commented May 1, 2013

it is, though I guess you mean that we should remove the 'components' command?

@ellisonbg
Copy link
Member

Oh sorry, yes, running bower install from that dir no longer makes sense.

@ellisonbg
Copy link
Member

I find the extra subdirectory structure external/components confusing and unnecessary. If it really is external then all of the other things should be in there as well (dateformat, pagedown, prettify). But if it is just components the extra subdir is not needed.

By dropping a .bowerrc file into the top level dir of the ipython-components repo with the contents:

{
  "directory": "."
}

It would cause the packges to be installed at the top-level of the repo, rather than in a components subdir. Then we could rename the submodule from from external to components and get rid of the extra level.

But all of this begs the question: do we intend this submodule to manage all externals or just the ones we have in there now?

@ellisonbg
Copy link
Member

The README.md in the submodule should have a clear list of the dependencies that it takes to run the update step (mainly the npm packages). It might even be nice to have a fabfile function that will run the npm install -g step for all of those packages. I don't want devs to have to dig through the different packages to discover what npm packages are required.

@ellisonbg
Copy link
Member

I see we are including jquery.ui 1.10.2 in the components dir. We need to add a comment to the README.md that we can't start to use this version yet because of our mess...

Oh crap, are we going to run into a deadlock in our development...

  1. To get rid of our hack branch of jquery.ui, we have to migrate the menu stuff to use bootstrap.
  2. Bootstrap requires a newer version of jquery than our hack branch jquery.ui requires.

What this means is that removing our hackbranch jquery.ui and transitioning to bootstrap have to be done together. I was hoping we could move more incrementally through this path.

@ellisonbg
Copy link
Member

I love how the nonbower stuff is handled! Very simple workflow that will make it easy for us to update and package managers to follow...

@minrk
Copy link
Member Author
minrk commented May 1, 2013

But all of this begs the question: do we intend this submodule to manage all externals or just the ones we have in there now?

I think we want all third-party code in here

@minrk
Copy link
Member Author
minrk commented May 1, 2013

that said - I think only third-party code that can be installed in this way should be there. That is, it should always work to rm -rf components && update components, which is how it works now. I'm not sure that works with prettify / pagedown / datefmt, and I didn't want to add code that we are about to remove to the new repo.

@minrk
Copy link
Member Author
minrk commented May 2, 2013

I've flattened the components repo

@ellisonbg
Copy link
Member

Woohooo! I love it. I did the following:

cd static/components
fab update
git status # no changes!

This means that indeed this give us a perfectly reproducible way of managing packages.

ellisonbg added a commit that referenced this pull request May 2, 2013
@ellisonbg
Merge pull request #3240 from minrk/submodules
cec85ef 
remove js components from the repo
@ellisonbg ellisonbg merged commit cec85ef into ipython:master May 2, 2013
@minrk minrk deleted the submodules branch March 31, 2014 23:36
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
@ellisonbg
Merge pull request ipython#3240 from minrk/submodules
24ea091 
remove js components from the repo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0
Development

Successfully merging this pull request may close these issues.
6 participants
@minrk @bfroehle @ahmadia @takluyver @juliantaylor @ellisonbg
0