search

somaticio / tensorflow.rb

tensorflow for ruby
BSD 3-Clause "New" or "Revised" License
827 stars 66 forks source link

Do CI on all branches #40

Closed nethsix closed 8 years ago

nethsix commented 8 years ago

DO NOT MERGE YET: Under review

If you merge, nothing bad happens

Update circle.yml:

nethsix commented 8 years ago

@chrhansen I have updated the circle.yml to test the code of the pull request instead of from the master (which is incorrect as you pointed out).

Please help me eyeball it when you have time. I have broken down the command line into parts here and sprinkled some comments to make it an easier read hopefully:

Thank you for all your help!

chrhansen commented 8 years ago

@nethsix thanks for the explanation - I don't have a lot of experience with Docker, though. I'm currently running docker run -e ... on my own machine, to see what the heck happens. In the meantime, the comments/questions I have are:

  1. For readability, would it be possible to split the docker run -e ... into separate lines (is this the newline command? && \), like how @geoffreylitt mentioned https://github.com/Arafatk/tensorflow.rb/pull/37#issuecomment-235784193?
  2. I might be mixing the wrong things, but shouldn't/couldn't /repos/ruby-tensorflow be updated to /repos/tensorflow.rb?
  3. Shouldn't the docker run -it ... in the README, https://github.com/Arafatk/tensorflow.rb#docker, also be updated?
  4. I guess my last comment would be to add badges like https://github.com/Arafatk/tensorflow.rb/pull/37#issuecomment-235807658 to the README :)

UPDATE:

$dev docker run -e CIRCLE_PROJECT_REPONAME=$CIRCLE_PROJECT_REPONAME -e CIRCLE_BRANCH=$CIRCLE_BRANCH -it nethsix/ruby-tensorflow-ubuntu:0.0.1.a /bin/bash -l -c "mkdir -p /repos/ruby-tensorflow/circle-ci && cd /repos/ruby-tensorflow/circle-ci && git clone $CIRCLE_REPOSITORY_URL && cd /repos/ruby-tensorflow/circle-ci/ruby-tensorflow && git fetch && git checkout $CIRCLE_BRANCH && git pull && bundle install && cd /repos/ruby-tensorflow/circle-ci/ruby-tensorflow/ext/sciruby/tensorflow_c && ruby extconf.rb && make && make install && cd /repos/ruby-tensorflow/circle-ci/ruby-tensorflow && bundle exec rake install && bundle exec rspec"
Unable to find image 'nethsix/ruby-tensorflow-ubuntu:0.0.1.a' locally
0.0.1.a: Pulling from nethsix/ruby-tensorflow-ubuntu

90d6565b970a: Pull complete 
40553bdb8474: Pull complete 
...
...
3b542ee25736: Pull complete 
5049651b65c8: Pull complete 
Digest: sha256:6697849985834adf31e725db5240352a26804f023f49691d8eb96ff07d7f8741
Status: Downloaded newer image for nethsix/ruby-tensorflow-ubuntu:0.0.1.a
You must specify a repository to clone.

I guess I should have specified CIRCLE-env variables, but for whatever it's worth, I don't have other comments than those above.

chrhansen commented 8 years ago

@nethsix @Arafatk once this PR is ready and merged, I think we can safely trash https://github.com/Arafatk/tensorflow.rb/pull/31 and thank @nethsix for saving us all from an untested repo!

nethsix commented 8 years ago

@chrhansen Thanks! Having another smart person (not necessarily an expert) to eyeball through things is always useful.

  1. I read @geoffreylitt 's suggestion, and have replied to it; basically docker does not save results of a run so for now I'm chaining those commands.
  2. /repos/ruby-tensorflow is the directory within the Docker image itself so I guess leaving it the name as ruby-tensorflow is fine. I remember your comment about something similar in the Dockerfile, which I will update, and rebuild.
  3. Yes! The badge. I've added in another local branch. I will move it to this same pull request.