Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements comprehensive tooling improvements to enhance the build system, CI workflows, and code maintainability for the ring_buffer library. The changes focus on standardizing configurations, improving version management, removing external dependencies, and refactoring the core implementation for better code quality.
Key Changes:
- Simplified dependency management: Removed dependency on embetech::utils library and the SemanticVersion API, streamlining the codebase to use string-based version reporting only
- Enhanced version tracking: Introduced git_utils.cmake for robust Git commit ID retrieval and updated version string format to include commit information (e.g., "1.0.2+commitid")
- CI/CD improvements: Pinned GitHub Actions to specific commit hashes for reproducibility, switched to ubuntu-latest runners, and added scheduled CI runs
Reviewed changes
Copilot reviewed 21 out of 23 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ring_buffer.c | Refactored null checking and simplified control flow; changed parameter naming |
| src/ring_buffer_version.c | Removed SemanticVersion struct support; simplified to string-only version API |
| src/include/embetech/ring_buffer.h | Removed SemanticVersion dependency and RingBuffer_GetVersion() function |
| src/CMakeLists.txt | Integrated git_utils.cmake for commit ID retrieval; removed embetech::utils dependency |
| cmake/git_utils.cmake | New utility function for retrieving Git commit IDs with configurable options |
| cmake/requirements.cmake | Removed embeutils dependency; standardized CPMAddPackage casing |
| cmake/install.cmake | Formatting updates for consistency |
| cmake/install_header_licenses.cmake | Updated to use VERSION.txt with file(STRINGS) |
| doc/CMakeLists.txt | Modernized Doxygen configuration with find_package approach |
| tests/CMakeLists.txt | Removed workaround for googletest issue |
| CMakeLists.txt | Changed VERSION file to VERSION.txt with file(STRINGS) |
| CMakePresets.json | Renamed presets for consistency (native-gcc); updated to version 8; adjusted compiler flags |
| .github/workflows/on_push.yml | Pinned action versions; switched to ubuntu-latest; updated preset references |
| .github/workflows/maintain_tags.yml | Pinned action version; removed token parameter |
| .github/workflows/generate_artifacts.yml | Pinned action versions; updated preset naming convention |
| .gitignore | Changed .cache pattern from recursive to root-only |
| .cspell.yml | Migrated from .yaml; expanded word list and ignore patterns |
| .cspell.yaml | Removed (replaced by .cspell.yml) |
| .cmake-format | Increased line width to 150; added CPMAddPackage command configuration |
| .clangd | Added -Wall flag to compile options |
| LICENSE.md | Added MIT License |
| VERSION.txt | New file containing version number 1.0.2 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements and refactorings to the build system, CI workflows, and codebase for better maintainability, reproducibility, and clarity. Key changes include updates to CMake presets and workflow files for more consistent naming and configuration, improvements to version and commit ID handling, and enhancements to documentation and spell checking configuration.
Build system and configuration improvements:
CMakePresets.jsonto use consistent naming (e.g.,native-gcc), updated build/test/install presets, and simplified compiler flags for better clarity and maintainability. [1] [2] [3] [4]src/CMakeLists.txtto use a newget_git_commit_idfunction fromcmake/git_utils.cmakefor robust commit ID retrieval, and changed version string definition to include commit ID (e.g.,"1.2.3+commitid"). Also removed dependency onembetech::utils. [1] [2] [3]VERSIONfile toVERSION.txtin CMake and install scripts for consistency. [1] [2]Continuous Integration and workflow updates:
ubuntu-latestrunners, and improved preset usage and artifact naming. Also added a scheduled CI run. [1] [2] [3]Documentation and spell checking enhancements:
doc/CMakeLists.txtto use modern CMake syntax and clarified aliases. [1] [2].cspell.yamlto.cspell.yml, adding more words and ignore rules for compiler flags and defines. [1] [2]Other notable changes:
LICENSE.mdfile with the MIT License..clangdand.cmake-formatfor improved diagnostics and formatting. [1] [2] [3]SemanticVersionstruct and related API fromring_buffer.h, simplifying version retrieval to a string only. [1] [2]