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

chore: added Dockerfile's and minor changes to rustfmt.toml #251

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

ch4r10t33r
Copy link

Added

  • Dockerfile
  • Dockerfile.cross
  • minor tweaks to rustfmt.toml

Copy link
Member

@KolbyML KolbyML left a comment

Choose a reason for hiding this comment

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

@KolbyML
Copy link
Member

KolbyML commented Mar 27, 2025

Also conventional commits should be used

https://www.conventionalcommits.org/en/v1.0.0/

https://gist.github.com/qoomon/5dfcdf8eec66a051ecd85625518cfd13

@ch4r10t33r
Copy link
Author

https://github.com/paradigmxyz/reth/blob/main/Dockerfile

Any reason to prefer to chef? I don't have a strong preference to anything, just curious.

@KolbyML
Copy link
Member

KolbyML commented Mar 27, 2025

https://github.com/paradigmxyz/reth/blob/main/Dockerfile

Any reason to prefer to chef? I don't have a strong preference to anything, just curious.

It makes building the Dockerfile after changes a lot quicker https://github.com/LukeMathWalker/cargo-chef?tab=readme-ov-file#benefits-vs-limitations

Do to how rust is, it is really hard to take advantage of DockerFile's native caching without it

@ch4r10t33r ch4r10t33r marked this pull request as draft March 27, 2025 21:24
@KolbyML
Copy link
Member

KolbyML commented Mar 27, 2025

oh I just noticed you marked the PR as draft, I will wait to continue reviewing it

@Vid201
Copy link
Contributor

Vid201 commented Mar 27, 2025

It would be useful to have docker-compose.yml file as well. Maybe moving everything to separate folder?

@ch4r10t33r
Copy link
Author

It would be useful to have docker-compose.yml file as well. Maybe moving everything to separate folder?

There is only one executable atm, no? why do we then need docker-compose?

@ch4r10t33r
Copy link
Author

oh I just noticed you marked the PR as draft, I will wait to continue reviewing it

yes, I'll let you know as soon as its ready.

@ch4r10t33r ch4r10t33r changed the title Added docker and minor changes to rustfmt.toml chore!: Added docker and minor changes to rustfmt.toml Mar 28, 2025
@ch4r10t33r ch4r10t33r requested a review from KolbyML March 30, 2025 12:32
@ch4r10t33r ch4r10t33r marked this pull request as ready for review March 30, 2025 12:32
@ch4r10t33r ch4r10t33r requested a review from syjn99 as a code owner March 30, 2025 12:32
Cargo.toml Outdated
@@ -1,20 +1,20 @@
[workspace]

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Copy link
Member

@KolbyML KolbyML left a comment

Choose a reason for hiding this comment

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

Left some feedback

.dockerignore Outdated
testing/ef_tests/
target/
*.data
*.tar.gz
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
*.tar.gz
*.tar.gz

image

new line

Dockerfile.cross Outdated
COPY ./dist/bin/$TARGETARCH/ream /usr/local/bin/ream

EXPOSE 8545 8546
ENTRYPOINT ["/usr/local/bin/ream"]
Copy link
Member

Choose a reason for hiding this comment

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

image

WORKDIR /app

LABEL org.opencontainers.image.source=https://github.com/reamlabs/ream
LABEL org.opencontainers.image.description="Ream is a modular, open-source Ethereum beam chain client."
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
LABEL org.opencontainers.image.description="Ream is a modular, open-source Ethereum beam chain client."
LABEL org.opencontainers.image.description="Ream is a modular, open-source Ethereum Beam Chain client."

Dockerfile.cross Outdated
# This image is meant to enable cross-architecture builds.
# It assumes the ream binary has already been
# compiled for `$TARGETPLATFORM` and moved to `./bin`.
FROM --platform=$TARGETPLATFORM ubuntu:22.04
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
FROM --platform=$TARGETPLATFORM ubuntu:22.04
FROM --platform=$TARGETPLATFORM ubuntu:24.04

@KolbyML KolbyML changed the title chore!: Added docker and minor changes to rustfmt.toml chore: added Dockerfile's and minor changes to rustfmt.toml Mar 30, 2025
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.

4 participants