search

ExchangeUnion / xud-docker

Streamlined setup of xud and all dependencies via docker 🐳
https://docs.exchangeunion.com
GNU Affero General Public License v3.0
5 stars 5 forks source link

Grpc #840

Closed kilrau closed 3 years ago

kilrau commented 3 years ago

Depends on https://github.com/ExchangeUnion/xud/pull/1605#issuecomment-751610425.

xud image needs to be changed back to latest before merging.

LePremierHomme commented 3 years ago

I switched the xud PR to the latest grpc-tools 1.10.0 so we'd need to change this again since it's referencing 1.9.1 still. But wouldn't it make sense not to pin ourselves to a specific version? I figure we can use sed to remove the entire line containing grpc-tools?

Agree.

Or, better yet, why don't we add the --production flag to npm install so that we skip dev dependencies altogether? I don't believe we need them for docker purposes, it would solve the issue with arm and grpc-tools and it should overall make our docker builds faster and lighter.

I thought that it was needed for these steps:

npm run compile
npm run compile:seedutil
sangaman commented 3 years ago

I thought that it was needed for these steps:

npm run compile
npm run compile:seedutil

Maybe I'm mistaken but it looks like that only has a Go dependency, not any dependency on our devDependencies. But the current change in this PR is fine, we can look into the --production flag separately.

kilrau commented 3 years ago

look into the --production flag separately

Please open an issue (or better PR) for that, otherwise it'll be forgotten