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 celotool dockerfile #1088

Merged
merged 3 commits into from
Sep 25, 2019
Merged

Fix celotool dockerfile #1088

merged 3 commits into from
Sep 25, 2019

Conversation

tkporter
Copy link
Contributor

Description

Adds contractkit & protocol dependencies to the celotool dockerfile. Otherwise builds would fail, because contractkit is required (and contractkit has protocol as a dependency)

Tested

Docker builds succeeded

Other changes

n/a

Related issues

n/a

Backwards compatibility

Yes

@ashishb
Copy link
Contributor

ashishb commented Sep 24, 2019

How big is the docker file now? Can you try docker inspect and report the new vs old results here?

@tkporter
Copy link
Contributor Author

Looking at GCR "virtual size":
new: 677.2 MB
old: 548.4 MB

:(
Any ideas for reducing size?

@codecov
Copy link

codecov bot commented Sep 25, 2019

Codecov Report

Merging #1088 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1088   +/-   ##
=======================================
  Coverage   66.45%   66.45%           
=======================================
  Files         256      256           
  Lines        7367     7367           
  Branches      491      427   -64     
=======================================
  Hits         4896     4896           
- Misses       2379     2381    +2     
+ Partials       92       90    -2
Flag Coverage Δ
#mobile 66.45% <ø> (ø) ⬆️
Impacted Files Coverage Δ
packages/mobile/src/utils/formatting.ts 87.5% <0%> (ø) ⬆️
packages/mobile/src/identity/reducer.ts 41.66% <0%> (ø) ⬆️

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 2967a7b...6628ae4. Read the comment docs.

@mcortesi
Copy link
Contributor

Looking at GCR "virtual size":
new: 677.2 MB
old: 548.4 MB

:(
Any ideas for reducing size?

@ashishb this could be "fixed" if we used "npm builds" inspect of copying the mono repo...
eg. use the contractkit deployed version.

@ashishb
Copy link
Contributor

ashishb commented Sep 25, 2019

Any ideas for reducing size?

Yes, do a multi-stage build - https://github.com/celo-org/celo-monorepo/blob/master/dockerfiles/cli/Dockerfile#L48

@tkporter tkporter added the automerge Have PR merge automatically when checks pass label Sep 25, 2019
@ashishb ashishb merged commit 68c2a42 into master Sep 25, 2019
@ashishb ashishb deleted the trevor/celotool-dockerfile-fix branch September 25, 2019 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants