Skip to content

Remove cmate in favour of native CMake + CPM - #432

Merged
mpkorstanje merged 31 commits into
mainfrom
cpp-remove-cmate-use-native-cmake-and-CPM
Jun 30, 2026
Merged

Remove cmate in favour of native CMake + CPM#432
mpkorstanje merged 31 commits into
mainfrom
cpp-remove-cmate-use-native-cmake-and-CPM

Conversation

@daantimmer

@daantimmer daantimmer commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

🤔 What's changed?

  • Removed custom cmate scripts and replaced it with native cmake instead
  • Use Cmake Package Manager (CPM) to (optionally) download nlohmann/json
  • Update cpp.hpp.erb codegen to indent properly
  • Update cpp.hpp.erb to treat all functions equal on where the return type should be located (same line instead of separate line)
  • Update codegen to write to separate folders. Separating custom maintained sources/headers and generated files to make the maintenance work more streamlined. (The manually maintained sources are easily overlooked)

⚡️ What's your motivation?

The custom cmate script adds no value and only confusion on maintaining the cpp codebase. Secondly its an unmaintained script by a third party.

As for the codegen, there were some inconsistencies.

🏷️ What kind of change is this?

  • 🏦 Refactoring/debt/DX (improvement to code design, tooling, etc. without changing behaviour)

♻️ Anything particular you want feedback on?

I am not too sure about the separation of generated vs non-generated code. It makes reading and maintaining a lot easier. However it does mean that this is a very large change because of all the file moves.

Regarding the checklist's #pragma once todo item. This is a highly debated topic in C++ land. However as a public library we should follow the core guidelines as closely as possible, and thus follow the rule: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf8-use-include-guards-for-all-header-files

I can, if preferred, do this in a separate pull request.

On that note I'll also add tooling (in a separate PR) to check for compliancy with the c++ core guidelines.

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.
  • Update supporting code to maintain a consistent coding style to be done in separate PR
  • Update test-cpp to actually use different clang and gcc versions to be done in separate PR
  • Replace #pragma once with proper include guards. to be done in separate PR
  • Tooling for checking core guidelines

This text was originally generated from a template, then edited by hand. You can modify the template here.

@daantimmer
daantimmer marked this pull request as ready for review June 26, 2026 07:53
@daantimmer
daantimmer force-pushed the cpp-remove-cmate-use-native-cmake-and-CPM branch from a71efc0 to 0866520 Compare June 26, 2026 14:27
@mpkorstanje
mpkorstanje self-requested a review June 26, 2026 21:30
@daantimmer

Copy link
Copy Markdown
Contributor Author

I've verified these changes do not affect cucumber/gherkin -> https://github.com/cucumber/gherkin/pull/633/checks and are fully backwards compatible.

@mpkorstanje mpkorstanje changed the title remove cmate in favour of native CMake + CPM Remove cmate in favour of native CMake + CPM Jun 30, 2026

@mpkorstanje mpkorstanje left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, but I don't quite understand the caching setup for Github Actions.

Comment thread .github/workflows/codeql-cpp.yaml
Comment thread .github/workflows/codeql-cpp.yaml
Comment thread .github/workflows/test-cpp.yml
@mpkorstanje
mpkorstanje merged commit 4348c56 into main Jun 30, 2026
@mpkorstanje
mpkorstanje deleted the cpp-remove-cmate-use-native-cmake-and-CPM branch June 30, 2026 22:38
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.

2 participants