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

images: Add support to override the image to use per language #47

Merged
merged 3 commits into from
Jul 5, 2017
Merged

images: Add support to override the image to use per language #47

merged 3 commits into from
Jul 5, 2017

Conversation

abeaumont
Copy link
Contributor

Fixes #43
I used a slightly different environment variable name which I think may be better.
I also preferred to handle the environment in the same place where CLI inputs are handled, and pass this information to the server explicitely.

abeaumont added 2 commits July 4, 2017 15:16

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@codecov
Copy link

codecov bot commented Jul 4, 2017

Codecov Report

Merging #47 into master will increase coverage by 0.16%.
The diff coverage is 58.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #47      +/-   ##
==========================================
+ Coverage   59.75%   59.91%   +0.16%     
==========================================
  Files          15       15              
  Lines         738      741       +3     
==========================================
+ Hits          441      444       +3     
  Misses        256      256              
  Partials       41       41
Impacted Files Coverage Δ
rest.go 58.13% <0%> (ø) ⬆️
grpc.go 71.42% <0%> (ø) ⬆️
common.go 100% <100%> (ø) ⬆️
server.go 38.46% <80%> (+1.2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5035ff...03cd9e9. Read the comment docs.

Copy link
Contributor

@juanjux juanjux left a comment

Choose a reason for hiding this comment

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

This will be great for debugging, thanks.

@abeaumont abeaumont mentioned this pull request Jul 4, 2017
"os"
"strings"

errors "srcd.works/go-errors.v0"
Copy link
Member

Choose a reason for hiding this comment

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

No need to alias this. The defined package is errors even if the last part of the import path is not.

image := strings.TrimSpace(fields[1])
logrus.Debugf("Overriding image for %s: %s", lang, image)
overrides[lang] = image
}
Copy link
Member

Choose a reason for hiding this comment

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

We usually put a blank line after every for/if block.

* Do not alias an import
* Add a blank line after a for loop
@abeaumont abeaumont merged commit c391d95 into bblfsh:master Jul 5, 2017
@abeaumont abeaumont deleted the feature/override-driver-images branch July 5, 2017 15:30
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

3 participants