Skip to content

Conversation

@juliannguyen4
Copy link
Collaborator

@juliannguyen4 juliannguyen4 commented Jan 9, 2026

This is to help with maintainability

Currently each of the list operations has a helper function to convert Python parameters into C and add the operation to an as_operations object. This PR consolidates all the helper functions into one to centralize the logic for error handling, converting parameters, and memory cleanup.

Extra changes:

  • Internal: fix bug where get_cdt_ctx() does not return an error code when an invalid argument type is passed to the cdt_ctx_list_index_create context

Known issues that existed beforehand:

The following operations do not validate that the values argument is a list type. There's currently no test code for this case

  • OP_LIST_GET_BY_VALUE_LIST
  • OP_LIST_REMOVE_BY_VALUE_LIST
  • OP_LIST_APPEND_ITEMS
  • OP_LIST_INSERT_ITEMS

@codecov-commenter
Copy link

codecov-commenter commented Jan 9, 2026

Codecov Report

❌ Patch coverage is 94.03670% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.82%. Comparing base (2d5c83f) to head (8478474).
⚠️ Report is 2 commits behind head on dev.

Files with missing lines Patch % Lines
src/main/client/cdt_list_operate.c 93.89% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #925      +/-   ##
==========================================
+ Coverage   83.31%   83.82%   +0.51%     
==========================================
  Files          99       99              
  Lines       14422    13981     -441     
==========================================
- Hits        12016    11720     -296     
+ Misses       2406     2261     -145     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

3 participants