Skip to content

feat: migrate CBOR module to core/ with Result-based API - #153

Merged
flyingrobots merged 7 commits into
mainfrom
migrate/cbor-to-core
Jul 8, 2025
Merged

feat: migrate CBOR module to core/ with Result-based API#153
flyingrobots merged 7 commits into
mainfrom
migrate/cbor-to-core

Conversation

@flyingrobots

Copy link
Copy Markdown
Owner

Summary

  • Migrated CBOR module from src/ to core/ with Result-based API
  • Created Result-based CBOR API in core/include/gitmind/cbor/cbor.h
  • Implemented core/src/cbor/cbor.c with comprehensive bounds checking
  • Added extensive test suite in core/tests/unit/test_cbor.c
  • Updated meson.build to include new CBOR module and tests
  • Deleted legacy CBOR files from src/ directory

Test plan

  • All unit tests passing
  • Zero warnings across 6 C23 compilers
  • Comprehensive bounds checking tested
  • Error handling verified

🤖 Generated with Claude Code

- Create Result-based CBOR API in core/include/gitmind/cbor/cbor.h
- Implement core/src/cbor/cbor.c with comprehensive bounds checking
- Add extensive test suite in core/tests/unit/test_cbor.c
- Update meson.build to include new CBOR module and tests
- Delete legacy CBOR files from src/ directory
- All tests passing with zero warnings across 6 C23 compilers

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@flyingrobots
flyingrobots force-pushed the migrate/cbor-to-core branch from 4a119dc to d6b6354 Compare July 8, 2025 22:05
flyingrobots and others added 6 commits July 8, 2025 15:09
- Fix global constant naming (CamelCase)
- Add missing stdint.h include
- Reduce cognitive complexity by extracting helper functions
- Replace magic numbers with named constants
- Fix else-after-return warnings
- All tests passing, zero warnings

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Break down read_uint_value into smaller focused functions
- Add missing result.h include
- Rename parameter to avoid swappable-parameters warning
- Each helper function now has simple, single responsibility
- All tests still passing

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Fix naming violations (CamelCase constants)
- Fix short parameter names (3+ characters)
- Add missing result.h includes
- Maintain test compatibility

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Fix swappable parameters by reordering function signatures
- Reduce cognitive complexity by extracting read_cbor_length helper
- Add bounds-checked memcpy suppressions for security warnings
- Update header files and all test calls to match new signatures
- All tests passing, cognitive complexity now under threshold

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Fixed swappable parameters in CBOR module read functions
- Added required POSIX feature test macros for time operations
- Fixed signed char conversion issues in ULID module
- Replaced magic numbers with named constants
- Fixed short parameter names in time.h interface
- Added proper NOLINT directives for system requirements
- Changed ULID DECODING array from int8_t to int for proper sign handling

All 20 warnings have been resolved. The codebase now passes
clang-tidy with zero warnings under maximum strictness.

Fixes #153

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Headers should not define feature test macros as they can conflict
with definitions in .c files. The macro is already properly defined
in time.c and ulid.c where needed.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@flyingrobots
flyingrobots merged commit d941a0b into main Jul 8, 2025
13 checks passed
@flyingrobots
flyingrobots deleted the migrate/cbor-to-core branch July 8, 2025 23:47
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.

1 participant