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

[MRG] Fix submodule check out #809

Merged
merged 7 commits into from Nov 7, 2019

Conversation

@davidbrochart
Copy link
Contributor

davidbrochart commented Nov 5, 2019

This should fix jupyterhub/binderhub#997.
Should I add a test?

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Nov 5, 2019

We have some tests for the git content provider here. It would be good to add a test that makes sure we don't checkout submodules from master when using a branch.

docs/source/changelog.rst Outdated Show resolved Hide resolved
@davidbrochart

This comment has been minimized.

Copy link
Contributor Author

davidbrochart commented Nov 5, 2019

We can actually clone and check out a specific branch at the same time:

git clone -b <branch> <remote_repo>
@manics

This comment has been minimized.

Copy link
Contributor

manics commented Nov 5, 2019

@davidanthoff You can clone branches but not commits jupyterhub/binderhub#997 (comment)

@davidanthoff

This comment has been minimized.

Copy link
Contributor

davidanthoff commented Nov 5, 2019

@davidanthoff You can clone branches but not commits jupyterhub/binderhub#997 (comment)

Was that meant for me? Probably not?

1 similar comment
@davidanthoff

This comment has been minimized.

Copy link
Contributor

davidanthoff commented Nov 5, 2019

@davidanthoff You can clone branches but not commits jupyterhub/binderhub#997 (comment)

Was that meant for me? Probably not?

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Nov 5, 2019

I think it was for the other David :)

@manics

This comment has been minimized.

Copy link
Contributor

manics commented Nov 5, 2019

Sorry, wrong David! No idea why that autocomplete came up, GitHub is normally pretty good about suggesting the most relevant user.

@davidbrochart

This comment has been minimized.

Copy link
Contributor Author

davidbrochart commented Nov 6, 2019

The errors in the CI don't seem to be related to my changes.

@davidbrochart

This comment has been minimized.

Copy link
Contributor Author

davidbrochart commented Nov 7, 2019

Do you think it can be merged?

@betatim betatim changed the title [WIP] Fix submodule check out [MRG] Fix submodule check out Nov 7, 2019
@betatim

This comment has been minimized.

Copy link
Member

betatim commented Nov 7, 2019

I'll take a look and the CI jobs have been restarted. (I was waiting for the title to change from WIP to MRG and hence had it on my low priority list.)

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Nov 7, 2019

The test that fails is passing origin/b8259dac9eb as the ref to the checkout command which doesn't work any more so we need to improve on that.

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Nov 7, 2019

This is the first test in https://github.com/jupyter/repo2docker/blob/master/tests/external/reproductions.repos.yaml

You should be able to run it locally with pytest -svx tests/external/reproductions.repos.yaml::"LIGO Gravitational Waves" (though I think because the two tests have the same name it might select the "other one" so we need to fix that up as well)

@betatim betatim merged commit 75f2ca4 into jupyter:master Nov 7, 2019
8 of 19 checks passed
8 of 19 checks passed
codecov/project 88.35% (-2.75%) compared to 0dba611
Details
jupyter.repo2docker Build #20191107.2 failed
Details
jupyter.repo2docker (Job conda)
Details
jupyter.repo2docker (Job external)
Details
jupyter.repo2docker (Job julia)
Details
jupyter.repo2docker (Job pipfile)
Details
jupyter.repo2docker (Job r)
Details
jupyter.repo2docker (Job stencila_py)
Details
jupyter.repo2docker (Job stencila_r)
Details
jupyter.repo2docker (Job unit)
Details
jupyter.repo2docker (Job venv)
Details
ci/dockercloud Your tests passed in Docker Cloud
Details
codecov/patch 37.5% of diff hit (target 20%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
default Workflow: default
Details
jupyter.repo2docker (Job base) Job base succeeded
Details
jupyter.repo2docker (Job dockerfile) Job dockerfile succeeded
Details
jupyter.repo2docker (Job lint) Job lint succeeded
Details
jupyter.repo2docker (Job nix) Job nix succeeded
Details
@betatim

This comment has been minimized.

Copy link
Member

betatim commented Nov 7, 2019

Merged! Should get deployed in the next few hours

@davidbrochart

This comment has been minimized.

Copy link
Contributor Author

davidbrochart commented Nov 7, 2019

Thanks!

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