Skip to content

Conversation

darosior
Copy link
Member

It was useful to me, so i cleaned it up and PR'd it in case others do not have a full Ruby toolchain installed, but do have Docker.

@darosior
Copy link
Member Author

(This is based on the CI config)

Copy link

@janb84 janb84 left a comment

Choose a reason for hiding this comment

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

ACK 9f7a431

PR adds dockerfile (based on ci) and edited readme in contrib with instruction so that the bitcoincore.org website can be run locally for easy testing of changes.

This is a valuable addition and makes testing the changes locally easier without installing all the dependencies on the base machine.

  • code reviewed ✅
  • followed instructions to run it locally ✅

Copy link
Member

@willcl-ark willcl-ark left a comment

Choose a reason for hiding this comment

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

ACK 9f7a431

Dockerfile is nice and tidy, and works well in testing.

Copy link
Contributor

@stickies-v stickies-v left a comment

Choose a reason for hiding this comment

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

Concept ACK, but I think incrementally building the site at runtime makes more sense than doing it at build time (see suggestion).

@janb84
Copy link

janb84 commented Oct 21, 2025

Concept ACK, but I think incrementally building the site at runtime makes more sense than doing it at build time (see suggestion).

Although I like the suggestions, I think this PR container is ment to behave like the "real website" and therefor is an (almost) duplicate of the CI .
(the CI can use an update but that's a good followup)

@darosior
Copy link
Member Author

Thanks @stickies-v for the rewrite review. I took all your suggestions to the Dockerfile, but let aside the suggested Makefile change.

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