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

cmake: Port PR30263 from the master branch #263

Merged
merged 1 commit into from
Jul 16, 2024
Merged

Conversation

hebasto
Copy link
Owner

@hebasto hebasto commented Jul 13, 2024

This PR ports CI-related changes from bitcoin#30263 and ensures that the minimum required CMake version is available.

Here are CMake versions across all changes:

  • pre-PR30263: CMake 3.22.1
  • post-PR30263: CMake 3.18.4, which is less than the minimum required 3.22
  • this PR: CMake 3.28.3

@hebasto hebasto added the port from autotools Ported from the main repository label Jul 13, 2024
@hebasto hebasto added this to the Ready for master milestone Jul 13, 2024
@hebasto
Copy link
Owner Author

hebasto commented Jul 13, 2024

Friendly ping @maflcko @m3dwards :)

@m3dwards
Copy link

ACK ec4c557

@hebasto
Copy link
Owner Author

hebasto commented Jul 15, 2024

Rebased.

Copy link

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -7,8 +7,8 @@
export LC_ALL=C.UTF-8

export CONTAINER_NAME=ci_native_nowallet_libbitcoinkernel
export CI_IMAGE_NAME_TAG="docker.io/debian:bullseye"
# Use minimum supported python3.9 and clang-16, see doc/dependencies.md
export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04"

Choose a reason for hiding this comment

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

It's not the same change?

Copy link

Choose a reason for hiding this comment

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

I presume it is needed to bump cmake. I was thinking about bumping python for 29.x, so this will go away in the future anyway. 🤷

Choose a reason for hiding this comment

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

It'd be good if the PR description explained. Rather than claiming to port changes but doing something different for unclear reasons.

Copy link
Owner Author

Choose a reason for hiding this comment

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

It'd be good if the PR description explained. Rather than claiming to port changes but doing something different for unclear reasons.

My apologies. The PR description has been updated.

@hebasto hebasto merged commit d3b57c3 into cmake-staging Jul 16, 2024
39 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
port from autotools Ported from the main repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants