feat: fetch tobieaslocker lib as external lib#315
feat: fetch tobieaslocker lib as external lib#315Naum Puroski (nomevas) wants to merge 4 commits intomainfrom
Conversation
✅
|
| Descriptor | Linter | Files | Fixed | Errors | Warnings | Elapsed time |
|---|---|---|---|---|---|---|
| ✅ ACTION | actionlint | 6 | 0 | 0 | 0.02s | |
| ✅ CPP | clang-format | 229 | 0 | 0 | 0 | 1.52s |
| ✅ DOCKERFILE | hadolint | 1 | 0 | 0 | 0.03s | |
| markdownlint | 6 | 3 | 19 | 0 | 1.08s | |
| ✅ MARKDOWN | markdown-table-formatter | 6 | 3 | 0 | 0 | 0.28s |
| ✅ REPOSITORY | git_diff | yes | no | no | 0.03s | |
| ✅ REPOSITORY | grype | yes | no | no | 38.65s | |
| ✅ REPOSITORY | ls-lint | yes | no | no | 0.07s | |
| ✅ REPOSITORY | secretlint | yes | no | no | 4.31s | |
| ✅ REPOSITORY | syft | yes | no | no | 1.53s | |
| ✅ REPOSITORY | trivy | yes | no | no | 13.12s | |
| ✅ REPOSITORY | trivy-sbom | yes | no | no | 0.14s | |
| ✅ REPOSITORY | trufflehog | yes | no | no | 5.59s | |
| lychee | 83 | 1 | 0 | 3.49s | ||
| ✅ YAML | prettier | 10 | 0 | 0 | 0 | 0.71s |
| ✅ YAML | v8r | 10 | 0 | 0 | 6.75s | |
| ✅ YAML | yamllint | 10 | 0 | 0 | 0.5s |
Detailed Issues
⚠️ SPELL / lychee - 1 error
[404] https://github.com/yourname/amp-cucumber-cpp-runner.git | Network error: Not Found
📝 Summary
---------------------
🔍 Total..........162
✅ Successful.....161
⏳ Timeouts.........0
🔀 Redirected.......0
👻 Excluded.........0
❓ Unknown..........0
🚫 Errors...........1
Errors in CONTRIBUTING.md
[404] https://github.com/yourname/amp-cucumber-cpp-runner.git | Network error: Not Found
⚠️ MARKDOWN / markdownlint - 19 errors
CHANGELOG.md:31 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Bug Fixes"]
CHANGELOG.md:37 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
CHANGELOG.md:44 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:51 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
CHANGELOG.md:59 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "⚠ BREAKING CHANGES"]
CHANGELOG.md:63 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:72 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:78 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
CHANGELOG.md:86 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "⚠ BREAKING CHANGES"]
CHANGELOG.md:90 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:95 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Bug Fixes"]
CHANGELOG.md:104 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:115 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
CHANGELOG.md:123 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "⚠ BREAKING CHANGES"]
CHANGELOG.md:127 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:152 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Bug Fixes"]
CHANGELOG.md:159 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
cmake/WindowsToolchainFilesProvidedBy.md:1 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "<https://github.com/MarkSchofi..."]
README.md:114 error MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
See detailed reports in MegaLinter artifacts
Your project could benefit from a custom flavor, which would allow you to run only the linters you need, and thus improve runtime performances. (Skip this info by defining FLAVOR_SUGGESTIONS: false)
- Documentation: Custom Flavors
- Command:
npx mega-linter-runner@9.4.0 --custom-flavor-setup --custom-flavor-linters ACTION_ACTIONLINT,CPP_CLANG_FORMAT,DOCKERFILE_HADOLINT,MARKDOWN_MARKDOWNLINT,MARKDOWN_MARKDOWN_TABLE_FORMATTER,REPOSITORY_GIT_DIFF,REPOSITORY_GRYPE,REPOSITORY_LS_LINT,REPOSITORY_SECRETLINT,REPOSITORY_SYFT,REPOSITORY_TRIVY,REPOSITORY_TRIVY_SBOM,REPOSITORY_TRUFFLEHOG,SPELL_LYCHEE,YAML_PRETTIER,YAML_YAMLLINT,YAML_V8R
Show us your support by starring ⭐ the repository
Naum Puroski (nomevas)
force-pushed
the
feature/fetch-tobiaslocker-base64
branch
from
55d9615 to
a444d26
Compare
|
There was a problem hiding this comment.
Pull request overview
This PR migrates the vendored tobiaslocker/base64 code to a FetchContent-managed external dependency, and wires it into the project’s dependency configuration.
Changes:
- Remove the vendored
tobiaslocker/base64header/docs/license from the repository. - Fetch
tobiaslocker/base64viaFetchContentand disable its unit tests. - Refactor top-level CMake options/defaults and add
find_package(base64 REQUIRED)for non-FetchContent builds; add a missing include needed byParameter.hpp.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| external/tobiaslocker/base64/include/base64.hpp | Removes vendored Base64 header (now expected to come from fetched dependency). |
| external/tobiaslocker/base64/README.md | Removes vendored README. |
| external/tobiaslocker/base64/LICENSE | Removes vendored license file. |
| external/tobiaslocker/base64/CMakeLists.txt | Switches Base64 integration to FetchContent. |
| cucumber_cpp/library/Parameter.hpp | Adds direct include for DefinitionRegistration used by the parameter registration macro. |
| CMakeLists.txt | Introduces CCR_STANDALONE option/default refactor and adds find_package(base64 REQUIRED) for non-fetch builds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| set(BASE64_ENABLE_TESTING OFF CACHE BOOL "Disable base64 unit tests" FORCE) | ||
| FetchContent_MakeAvailable(tobiaslocker_base64) | ||
|
|
There was a problem hiding this comment.
FetchContent_MakeAvailable(tobiaslocker_base64) does not guarantee that an un-namespaced target called base64 exists. The rest of the project links against base64 (e.g., cucumber_cpp/library/engine/CMakeLists.txt), so if the fetched project exports only a namespaced target (like base64::base64) configuration/linking will fail. Consider adding a small compatibility shim here that creates an alias/INTERFACE target named base64 when only a namespaced target is provided (and/or vice versa), so both fetch and non-fetch dependency modes work consistently.
| # Compatibility shim: ensure both `base64` and `base64::base64` targets exist. | |
| # Some parts of the project link against `base64`, while the fetched project | |
| # may only export a namespaced target (e.g. `base64::base64`), or vice versa. | |
| if (TARGET base64::base64 AND NOT TARGET base64) | |
| add_library(base64 INTERFACE) | |
| target_link_libraries(base64 INTERFACE base64::base64) | |
| elseif (TARGET base64 AND NOT TARGET base64::base64) | |
| add_library(base64::base64 INTERFACE) | |
| target_link_libraries(base64::base64 INTERFACE base64) | |
| endif() |
| find_package(cucumber_gherkin REQUIRED) | ||
| find_package(fmt REQUIRED) | ||
| find_package(base64 REQUIRED) | ||
|
|
There was a problem hiding this comment.
find_package(base64 REQUIRED) relies on the external Base64 package to provide a CMake package config and (given current target_link_libraries(... base64 ...) usage) a target that is compatible with the un-namespaced base64 target name used throughout the repo. To avoid fragile builds across different package exports, add a post-find_package check that maps the package's exported target(s) to the expected base64 target name (e.g., create an ALIAS/INTERFACE wrapper when only base64::base64 exists).
| # Normalize base64 target name to un-namespaced 'base64' used in this project | |
| if (NOT TARGET base64) | |
| if (TARGET base64::base64) | |
| add_library(base64 INTERFACE IMPORTED) | |
| target_link_libraries(base64 INTERFACE base64::base64) | |
| else() | |
| message(FATAL_ERROR | |
| "The 'base64' package was found, but it does not provide a compatible " | |
| "CMake target named 'base64' or 'base64::base64'.") | |
| endif() | |
| endif() |
| if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) | ||
| set(CCR_STANDALONE On) | ||
| set(CCR_DEFAULTOPT On) | ||
| endif() | ||
|
|
||
| option(CCR_STANDALONE "Build cucumber-cpp-runner as a standalone project" ${CCR_DEFAULTOPT}) | ||
| option(CCR_FETCH_DEPS "Fetch dependencies via FetchContent." ${CCR_DEFAULTOPT} ) | ||
| option(CCR_BUILD_TESTS "Enable build of the tests" ${CCR_DEFAULTOPT}) | ||
| option(CCR_ENABLE_COVERAGE "Enable compiler flags for code coverage measurements" Off) | ||
| option(CCR_ENABLE_TIME_PROFILE "Enable compiler flags for time profiling measurements" Off) |
There was a problem hiding this comment.
CCR_DEFAULTOPT is only set when this repository is the top-level project; when used as a subproject it expands to an empty string in the subsequent option(...) calls. While CMake treats that as OFF, it’s implicit and makes the defaulting behavior harder to reason about. Consider explicitly setting CCR_DEFAULTOPT Off in the non-standalone case to keep option defaults unambiguous and consistent.
No description provided.