Patrick Steinhardt <ps@xxxxxx> writes:
> We have a bunch of infrastructure wired up that allows us to print
> reftable records, tables and stacks. While this functionality is wired
> up via various "test-tool reftable" options, it is never used. It also
> feels kind of dubious whether any other eventual user of the reftable
> library should use it as it very much feels like a debugging aid rather
> than something sensible. The format itself is somewhat inscrutable and
> the infrastructure is non-extensible.
>
> Drop this code. The only remaining function in this context is
> `reftable_reader_print_blocks()`, which we do use in our tests.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> reftable/dump.c | 16 +----
> reftable/generic.c | 47 -------------
> reftable/reader.c | 21 ------
> reftable/record.c | 127 ------------------------------------
> reftable/record.h | 4 --
> reftable/reftable-generic.h | 3 -
> reftable/reftable-reader.h | 2 -
> reftable/reftable-record.h | 8 ---
> reftable/reftable-stack.h | 3 -
> reftable/stack.c | 20 ------
> reftable/stack_test.c | 7 --
> 11 files changed, 1 insertion(+), 257 deletions(-)
>
> diff --git a/reftable/dump.c b/reftable/dump.c
> index 2953e0a83a..35a1731da9 100644
> --- a/reftable/dump.c
> +++ b/reftable/dump.c
> @@ -41,9 +41,6 @@ int reftable_dump_main(int argc, char *const *argv)
> {
> int err = 0;
> int opt_dump_blocks = 0;
> - int opt_dump_table = 0;
> - int opt_dump_stack = 0;
> - uint32_t opt_hash_id = GIT_SHA1_FORMAT_ID;
> const char *arg = NULL, *argv0 = argv[0];
>
> for (; argc > 1; argv++, argc--)
> @@ -51,12 +48,6 @@ int reftable_dump_main(int argc, char *const *argv)
> break;
> else if (!strcmp("-b", argv[1]))
> opt_dump_blocks = 1;
> - else if (!strcmp("-t", argv[1]))
> - opt_dump_table = 1;
> - else if (!strcmp("-6", argv[1]))
> - opt_hash_id = GIT_SHA256_FORMAT_ID;
> - else if (!strcmp("-s", argv[1]))
> - opt_dump_stack = 1;
> else if (!strcmp("-?", argv[1]) || !strcmp("-h", argv[1])) {
> print_help();
> return 2;
> @@ -70,13 +61,8 @@ int reftable_dump_main(int argc, char *const *argv)
I'm a bit skeptical about this change because I definitely have used the
`-t` and `-s` options a bunch of times to understand what a table holds.
Since the reftable format is binary, this is the only tooling we have
which allows us to read this format from a plumbing point of view. I'd
keep them. I guess the stack printing just iterates over the tables and
prints them and could be removed, but I'd keep the option to dump a
table.
Also this patch misses cleaning up `print_help` if we go down this
route.
[snip]
Attachment:
signature.asc
Description: PGP signature
