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: restore makefile commands #82

Merged
merged 3 commits into from
Apr 6, 2020
Merged

fix: restore makefile commands #82

merged 3 commits into from
Apr 6, 2020

Conversation

lorenzo-cavazzi
Copy link
Member

fix #81

Makefile Outdated Show resolved Hide resolved
@emmjab
Copy link
Contributor

emmjab commented Apr 3, 2020

Questions:

  1. So the CI no longer uses the Makefile (when we switched from travis to github actions), but I get that it's still useful to keep the makefile for local builds & tests.

  2. The naming conventions on the variables has been updated a bunch in the github actions, and in order to keep this maintainable it might make sense to update them in the makefile as well.

  3. Is there a way to run the github actions locally instead? Maybe this question is backwards and we should just run the makefile from the github action.

  4. more of a side-note: the multi-step builds haven't been fixed yet in the github actions (i.e. the 2 cuda builds that rely on "base" which is now py3.7); mostly because I can't see any workaround for duplicated code (but I see the Makefile also uses the duplicated codeblock, so maybe that's the way to go...).

@emmjab emmjab self-requested a review April 6, 2020 09:36
Copy link
Contributor

@emmjab emmjab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok in order to move forward with updating the builds for this repo, I'm going to move forward with merging this! Tested this by building with make all and make <specific extension> locally:

For documentation purposes, things not covered in this PR:

  • missing the new julia extension
  • variables don't match github actions (will fix in separate PR)
  • repo name for dockerhub doesn't match github actions (will fix in separate PR; maybe this naming is better)
  • cuda build had issues but expected
  • all the other builds were fine

@emmjab emmjab merged commit 408e107 into master Apr 6, 2020
@lorenzo-cavazzi lorenzo-cavazzi deleted the 81-fix-makefile branch April 6, 2020 09:43
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.

Fix Makefile or Readme
2 participants