[CLIENT-3106] Remove dead code in conversions.c #817

juliannguyen4 · 2025-08-18T23:27:07Z

record_to_resultpyobject() was a helper function for client.batch_get_ops(), which is now removed.
record_to_pyobject_cnvt_list_to_map() and as_list_of_map_to_py_tuple_list(): these were helper functions that were used before Python client version 2.1.3 to return the result of certain map operations. They are no longer used starting from Python client 2.1.3 and higher, so it is safe to remove
bin_strict_type_checking() isn't used anywhere. But it would be good to consolidate the bin checking code into one place, since it is currently spread out all over the codebase
as_batch_read_results_to_pyobject() was used by get_many() which has been removed
batch_read_records_to_pyobject() was used by select_many() which has been removed

Extra Changes

Merge do_*_to_pyobject() methods into their calling methods, since they have the same function signature

Manual testing

Valgrind shows no memory errors or leaks
Build artifacts passes. Two failing jobs show no test failures, so not sure why they show up as errors
Massif usage looks the same

codecov-commenter · 2025-08-18T23:51:11Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.25%. Comparing base (4aea102) to head (c4f699f).
⚠️ Report is 4 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #817      +/-   ##
==========================================
+ Coverage   82.45%   83.25%   +0.80%     
==========================================
  Files          99       99              
  Lines       14496    14347     -149     
==========================================
- Hits        11952    11945       -7     
+ Misses       2544     2402     -142

☔ 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.

…ead-code

DomPeliniAerospike

Left just one suggestion, everything else looks good.

DomPeliniAerospike · 2025-11-11T13:36:13Z

src/main/conversions.c

        as_list *l = as_list_fromval((as_val *)val);
        if (l) {
            PyObject *py_list = NULL;
-            if (cnvt_list_to_map) {


Since as_list_of_map_to_py_tuple_list is no longer used, you should also remove usage of cnvt_list_to_map variable.

cnvt_list_to_map is used in several function signatures, but shouldn't be necessary since as_list_of_map_to_py_tuple_list is removed.

There should be more dead code linked to cnvt_list_to_map.

If this is outside of the scope for this PR, then we should log a ticket for removing this and dead code which set this funciton parameter to true in order to reduce complexity when stepping through functions.

I forgot to remove it, thanks for reminding me

…meters

…sed parameters" This reverts commit 54d28a7.

…rameters

…rimental: enable -Wextra to see if this complains about unused parameters" This reverts commit 70229b9.

DomPeliniAerospike

LGTM!

juliannguyen4 · 2025-11-11T21:39:28Z

no memory errors or leaks from changes
massif mem usage looks ok

juliannguyen4 · 2025-11-14T00:26:47Z

Currently holding off on merging this because I noticed the server crashing in one macos arm64 job. I'd like to investigate this further, after IT upgrades the self hosted mac runners to macos 14

…ead-code

juliannguyen4 · 2025-11-18T20:53:26Z

Build artifacts passes except for noise. I believe the server was crashing due to IT maintenance

* [CLIENT-3793] Remove macOS 13 support (#846) * Auto-bump version to 18.1.0rc3.dev1 [skip ci] * [CLIENT-3106] Remove dead code in conversions.c (#817) - record_to_resultpyobject() was a helper function for client.batch_get_ops(), which is now removed. - record_to_pyobject_cnvt_list_to_map() and as_list_of_map_to_py_tuple_list(): these were helper functions that were used before Python client version 2.1.3 to return the result of certain map operations. They are no longer used starting from Python client 2.1.3 and higher, so it is safe to remove - bin_strict_type_checking() isn't used anywhere. But it would be good to consolidate the bin checking code into one place, since it is currently spread out all over the codebase - as_batch_read_results_to_pyobject() was used by get_many() which has been removed - batch_read_records_to_pyobject() was used by select_many() which has been removed Extra Changes Merge do_*_to_pyobject() methods into their calling methods, since they have the same function signature * Auto-bump version to 18.1.0rc3.dev2 [skip ci] --------- Co-authored-by: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

clean up dead code

c2b6dba

juliannguyen4 added 7 commits October 14, 2025 14:57

Merge remote-tracking branch 'origin/dev' into CLIENT-3106-clean-up-d…

1887047

…ead-code

unused declaration

41803b0

Merge remote-tracking branch 'origin/dev' into CLIENT-3106-clean-up-d…

25b150e

…ead-code

Remove dead code that was only used prior to python client 2.1.3

fc7e746

rm bin_strict_type_checking

c48dd48

Rm

1c95dec

Rm

79f462a

juliannguyen4 changed the title ~~[CLIENT-3106] Clean up dead code~~ [CLIENT-3106] Remove dead code Nov 10, 2025

juliannguyen4 marked this pull request as ready for review November 10, 2025 22:46

juliannguyen4 requested a review from DomPeliniAerospike November 10, 2025 22:47

juliannguyen4 changed the title ~~[CLIENT-3106] Remove dead code~~ [CLIENT-3106] Remove dead code in conversions.c Nov 10, 2025

DomPeliniAerospike requested changes Nov 11, 2025

View reviewed changes

juliannguyen4 added 7 commits November 11, 2025 08:40

Rm unused decl

9caab14

Rm unused parameter.

9443032

Collapse do_.*_to_pyobject methods into the methods that call them

fbf0819

Experimental: enable -Wall to see if this complains about unused para…

54d28a7

…meters

Revert "Experimental: enable -Wall to see if this complains about unu…

151304d

…sed parameters" This reverts commit 54d28a7.

Experimental: enable -Wextra to see if this complains about unused pa…

70229b9

…rameters

C client contains warnings we don't want to fix for now. Revert "Expe…

fb81256

…rimental: enable -Wextra to see if this complains about unused parameters" This reverts commit 70229b9.

juliannguyen4 requested a review from DomPeliniAerospike November 11, 2025 17:15

DomPeliniAerospike approved these changes Nov 11, 2025

View reviewed changes

Merge remote-tracking branch 'origin/dev' into CLIENT-3106-clean-up-d…

c4f699f

…ead-code

juliannguyen4 merged commit 3b457ef into dev Nov 18, 2025
127 of 130 checks passed

juliannguyen4 deleted the CLIENT-3106-clean-up-dead-code branch November 18, 2025 20:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[CLIENT-3106] Remove dead code in conversions.c #817

[CLIENT-3106] Remove dead code in conversions.c #817

Uh oh!

juliannguyen4 commented Aug 18, 2025 •

edited

Loading

Uh oh!

codecov-commenter commented Aug 18, 2025 •

edited

Loading

Uh oh!

DomPeliniAerospike left a comment

Uh oh!

DomPeliniAerospike Nov 11, 2025

Uh oh!

juliannguyen4 Nov 11, 2025

Uh oh!

DomPeliniAerospike left a comment

Uh oh!

juliannguyen4 commented Nov 11, 2025

Uh oh!

juliannguyen4 commented Nov 14, 2025

Uh oh!

juliannguyen4 commented Nov 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[CLIENT-3106] Remove dead code in conversions.c #817

[CLIENT-3106] Remove dead code in conversions.c #817

Uh oh!

Conversation

juliannguyen4 commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Extra Changes

Manual testing

Uh oh!

codecov-commenter commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DomPeliniAerospike left a comment

Choose a reason for hiding this comment

Uh oh!

DomPeliniAerospike Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

juliannguyen4 Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

DomPeliniAerospike left a comment

Choose a reason for hiding this comment

Uh oh!

juliannguyen4 commented Nov 11, 2025

Uh oh!

juliannguyen4 commented Nov 14, 2025

Uh oh!

juliannguyen4 commented Nov 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

juliannguyen4 commented Aug 18, 2025 •

edited

Loading

codecov-commenter commented Aug 18, 2025 •

edited

Loading