| Message ID | cover.1724308389.git.ps@pks.im (mailing list archive) |
|---|---|
| Headers | show |
| Series | reftable: drop generic `reftable_table` interface | expand |
Patrick Steinhardt <ps@pks.im> writes: [snip] > Range-diff against v2: > 1: 472c169b501 = 1: 472c169b501 reftable/merged: expose functions to initialize iterators > 2: bc6f1cd8c1b = 2: bc6f1cd8c1b reftable/merged: rename `reftable_new_merged_table()` > 3: 58e91ab4b34 = 3: 58e91ab4b34 reftable/merged: stop using generic tables in the merged table > 4: 6ba3fcee411 = 4: 6ba3fcee411 reftable/stack: open-code reading refs > 5: cac08a934c5 = 5: cac08a934c5 reftable/iter: drop double-checking logic > 6: 103262dc79c = 6: 103262dc79c reftable/generic: move generic iterator code into iterator interface > 7: 4011fa65d81 = 7: 4011fa65d81 reftable/dump: drop unused `compact_stack()` > 8: ceaa296bfd4 = 8: ceaa296bfd4 t/helper: inline `reftable_dump_main()` > 9: a62e4612e97 = 9: a62e4612e97 t/helper: inline `reftable_reader_print_file()` > 10: 7acfe4fecc5 ! 10: 242c179df5f t/helper: inline `reftable_stack_print_directory()` > @@ Commit message > Move `reftable_stack_print_directory()` into the "dump-reftable" helper. > This follows the same reasoning as the preceding commit. > > + Note that this requires us to remove the tests for this functionality in > + `reftable/stack_test.c`. The test does not really add much anyway, > + because all it verifies is that we do not crash or run into an error, > + and it specifically doesn't check the outputted data. Also, as the code > + is now part of the test helper, it doesn't make much sense to have a > + unit test for it in the first place. > + > Signed-off-by: Patrick Steinhardt <ps@pks.im> > > ## reftable/reftable-stack.h ## > 11: 8bd53a1a656 = 11: a05e2060996 t/helper: inline `reftable_table_print()` > 12: c50aabbb804 = 12: ee22a08e11e t/helper: inline printing of reftable records > 13: 5498395872c = 13: 0a3c619e842 t/helper: use `hash_to_hex_algop()` to print hashes > 14: 5390be75c37 = 14: 8eab399dfc6 t/helper: refactor to not use `struct reftable_table` > 15: 5aeab8ee077 = 15: b5d7b5679b5 reftable/generic: drop interface > -- > 2.46.0.164.g477ce5ccd6.dirty This was the only change I requested in the previous version. So looks good to me now! Thanks Karthik
Patrick Steinhardt <ps@pks.im> writes: > this is the third version of my patch series that gets rid of the > generic `reftable_table` interface. It made it way harder to understand > the reftable code base and is not really required nowadays anymore where > we have generic re-seekable reftable iterators. Looking good. Let me mark it for 'next'. Thanks.
Hi, this is the third version of my patch series that gets rid of the generic `reftable_table` interface. It made it way harder to understand the reftable code base and is not really required nowadays anymore where we have generic re-seekable reftable iterators. There is only a single change compared to v2, which updates one of the commit messages to explain why it is fine to drop tests for the printing functionality. This patch series continues to build on top of 25673b1c47 (The third batch, 2024-08-07) with Junio's ps/reftable-stack-compaction at f234df07f6 (reftable/stack: handle locked tables during auto-compaction, 2024-08-08) merged into it. Thanks! Patrick Patrick Steinhardt (15): reftable/merged: expose functions to initialize iterators reftable/merged: rename `reftable_new_merged_table()` reftable/merged: stop using generic tables in the merged table reftable/stack: open-code reading refs reftable/iter: drop double-checking logic reftable/generic: move generic iterator code into iterator interface reftable/dump: drop unused `compact_stack()` t/helper: inline `reftable_dump_main()` t/helper: inline `reftable_reader_print_file()` t/helper: inline `reftable_stack_print_directory()` t/helper: inline `reftable_table_print()` t/helper: inline printing of reftable records t/helper: use `hash_to_hex_algop()` to print hashes t/helper: refactor to not use `struct reftable_table` reftable/generic: drop interface Makefile | 2 - reftable/dump.c | 111 --------------- reftable/generic.c | 229 ------------------------------- reftable/generic.h | 37 ----- reftable/iter.c | 126 ++++++++++++++--- reftable/iter.h | 30 +++- reftable/merged.c | 72 ++++------ reftable/merged.h | 4 +- reftable/reader.c | 70 +--------- reftable/reader.h | 4 + reftable/record.c | 127 ----------------- reftable/record.h | 1 - reftable/reftable-generic.h | 47 ------- reftable/reftable-merged.h | 26 ++-- reftable/reftable-reader.h | 9 -- reftable/reftable-record.h | 8 -- reftable/reftable-stack.h | 3 - reftable/reftable-tests.h | 1 - reftable/stack.c | 94 ++++++------- reftable/stack_test.c | 29 ++-- t/helper/test-reftable.c | 189 ++++++++++++++++++++++++- t/unit-tests/t-reftable-merged.c | 17 +-- 22 files changed, 422 insertions(+), 814 deletions(-) delete mode 100644 reftable/dump.c delete mode 100644 reftable/generic.c delete mode 100644 reftable/generic.h delete mode 100644 reftable/reftable-generic.h Range-diff against v2: 1: 472c169b501 = 1: 472c169b501 reftable/merged: expose functions to initialize iterators 2: bc6f1cd8c1b = 2: bc6f1cd8c1b reftable/merged: rename `reftable_new_merged_table()` 3: 58e91ab4b34 = 3: 58e91ab4b34 reftable/merged: stop using generic tables in the merged table 4: 6ba3fcee411 = 4: 6ba3fcee411 reftable/stack: open-code reading refs 5: cac08a934c5 = 5: cac08a934c5 reftable/iter: drop double-checking logic 6: 103262dc79c = 6: 103262dc79c reftable/generic: move generic iterator code into iterator interface 7: 4011fa65d81 = 7: 4011fa65d81 reftable/dump: drop unused `compact_stack()` 8: ceaa296bfd4 = 8: ceaa296bfd4 t/helper: inline `reftable_dump_main()` 9: a62e4612e97 = 9: a62e4612e97 t/helper: inline `reftable_reader_print_file()` 10: 7acfe4fecc5 ! 10: 242c179df5f t/helper: inline `reftable_stack_print_directory()` @@ Commit message Move `reftable_stack_print_directory()` into the "dump-reftable" helper. This follows the same reasoning as the preceding commit. + Note that this requires us to remove the tests for this functionality in + `reftable/stack_test.c`. The test does not really add much anyway, + because all it verifies is that we do not crash or run into an error, + and it specifically doesn't check the outputted data. Also, as the code + is now part of the test helper, it doesn't make much sense to have a + unit test for it in the first place. + Signed-off-by: Patrick Steinhardt <ps@pks.im> ## reftable/reftable-stack.h ## 11: 8bd53a1a656 = 11: a05e2060996 t/helper: inline `reftable_table_print()` 12: c50aabbb804 = 12: ee22a08e11e t/helper: inline printing of reftable records 13: 5498395872c = 13: 0a3c619e842 t/helper: use `hash_to_hex_algop()` to print hashes 14: 5390be75c37 = 14: 8eab399dfc6 t/helper: refactor to not use `struct reftable_table` 15: 5aeab8ee077 = 15: b5d7b5679b5 reftable/generic: drop interface