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

FIX: Trim Python package requirements #35

Merged

Conversation

josephmje
Copy link
Contributor

@josephmje josephmje commented Dec 1, 2020

Addresses #31

@jhlegarreta
Copy link
Contributor

The change looks fair to me.

Not sure why the CIs are not being triggered, though. Not specific to this PR, since I realize that others have not triggered them; only merges to the main branch have triggered them, even thought they are set for push events and should be enough. Maybe it has to do with triggering the GitHub Actions by pushes from forked repositories.

@jhlegarreta jhlegarreta self-requested a review December 1, 2020 15:18
@josephmje
Copy link
Contributor Author

it looks like the install requirements step is now passing. but the test notebooks step is failing. does the path for the ipynb notebooks need to be updated?

@jhlegarreta
Copy link
Contributor

it looks like the install requirements step is now passing. but the test notebooks step is failing. does the path for the ipynb notebooks need to be updated?

OK, I see the build in your fork. Probably the search pattern I put was relative the current directory and it should be adjusted. We could address that in a separate PR, and have this merged to make things go forward (and since in any case the downloading the data is a step that still requires to be added to the CI pipeline).

@josephmje
Copy link
Contributor Author

Sounds good! I'll merge this is and the CI pipeline can be updated separately.

@josephmje josephmje merged commit 26f7e59 into carpentries-incubator:master Dec 1, 2020
@josephmje josephmje deleted the fix/trim_requirements branch December 22, 2020 14:16
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