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
Refactor .travis/install.sh. #1443
Conversation
Test PASSed. |
;; | ||
py27) | ||
curl -O https://bootstrap.pypa.io/get-pip.py | ||
sudo python get-pip.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing a ;;
here
Test FAILed. |
fi | ||
fi | ||
if [[ "${OPENSSL}" == "0.9.8" ]]; then | ||
brew upgrade openssl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing a fi
Test PASSed. |
Test PASSed. |
Test PASSed. |
Looks like you need to add a flag to do non-interactive ppa addition: https://travis-ci.org/pyca/cryptography/jobs/39076978 Messing with our install scripts is difficult :) |
if which pyenv > /dev/null; then | ||
eval "$(pyenv init -)" | ||
eval "$(pyenv init -)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new indent level here looks wrong.
Test PASSed. |
Test PASSed. |
Test PASSed. |
There seems to be a loss of coverage here somehow; not sure what to make of that. Can you try merging master in? If that does it fix it, I think it means this introduced a bug effecting which jobs are run, which means I didn't do a good enough review (because otherwise this looks like a solid cleanup!) |
Test PASSed. |
Hmm, coveralls is still reporting a missing line... I have no idea where that's from though... |
Test PASSed. |
docs) | ||
curl -O https://bootstrap.pypa.io/get-pip.py | ||
sudo python get-pip.py | ||
;; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this changes the way that we indent case
statements, is there a reason for this?
Test PASSed. |
So there's still a coverage issue here, and coveralls is showing 100% on master, so I think it's probably real :-( The only way that could happen, I think, is if this was changing what jobs were actually running, so I think this needs a bit closer review. It might be useful if we were able to figure out which lines weren't covered? |
starts digging through travis' logs |
Test PASSed. |
Refactor .travis/install.sh.
Fixes #1439.
Probably a few more bits that can be refactored but it's a start.