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

Detect Fedora properly in install_deps.sh #5513

Merged
merged 1 commit into from
Nov 29, 2018

Conversation

corollari
Copy link
Contributor

@corollari corollari commented Nov 27, 2018

Description

Make the double quotes around the distro name optional so it matches properly in Fedora (NAME=Fedora)

Fixes #5512

Checklist

  • Code compiles correctly
  • All tests are passing
  • New tests have been created which fail without the change (if possible)
  • README / documentation was extended, if necessary
  • Changelog entry (if change is visible to the user)
  • Used meaningful commit messages

@corollari
Copy link
Contributor Author

It seems the Travis test is failing because it's trying to run the script in a docker image, would it be possible to know which docker image it's pulling? I've looked at the test directory and can't find it there.

scripts/install_deps.sh Outdated Show resolved Hide resolved
@chriseth
Copy link
Contributor

The docker image is docker pull trzeci/emscripten:sdk-tag-1.37.21-64bit - but how would that be relevant?

@corollari
Copy link
Contributor Author

corollari commented Nov 28, 2018

The docker image is docker pull trzeci/emscripten:sdk-tag-1.37.21-64bit - but how would that be relevant?

Given that the Travis CI is not a required test maybe it's not needed but I thought I'd have to fix the issues that made the travis test fail before merging

@chriseth chriseth changed the title Fix #5512 - Detect Fedora properly in install_deps.sh Detect Fedora properly in install_deps.sh Nov 28, 2018
@codecov
Copy link

codecov bot commented Nov 28, 2018

Codecov Report

Merging #5513 into develop will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5513      +/-   ##
===========================================
+ Coverage    88.14%   88.19%   +0.05%     
===========================================
  Files          319      319              
  Lines        31672    31649      -23     
  Branches      3802     3791      -11     
===========================================
- Hits         27917    27914       -3     
+ Misses        2468     2459       -9     
+ Partials      1287     1276      -11
Flag Coverage Δ
#all 88.19% <ø> (+0.05%) ⬆️
#syntax 28.97% <ø> (ø) ⬆️

@chriseth
Copy link
Contributor

Thanks!
Waiting for travis to run and will squash and merge afterwards.

Make the double quotes around the distro name optional so it matches properly in Fedora (`NAME=Fedora`)
@chriseth
Copy link
Contributor

Sorry, it was still not working, had to change it again. Fingers crossed!

@chriseth chriseth merged commit 6b11ef1 into ethereum:develop Nov 29, 2018
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.

None yet

2 participants