karthik nayak <karthik.188@xxxxxxxxx> writes: > Seems like the CI caught it too [1]. > >> + it->ops->close(it->iter_arg); >> + it->ops = NULL; >> + FREE_AND_NULL(it->iter_arg); >> +} >> + > > [snip] > > [1]: https://gitlab.com/gitlab-org/git/-/jobs/7563308943 Looking at it, the suggestions the CI job makes look hit-and-miss. For examle, this one -static int stack_compact_range_stats(struct reftable_stack *st, - size_t first, size_t last, +static int stack_compact_range_stats(struct reftable_stack *st, size_t first, + size_t last, struct reftable_log_expiry_config *config, unsigned int flags) is a degradation of readability. "first" and "last" pretty much act as a pair and they read better sitting together next to each other. The rewrite is reducing neither the total number of lines or the maximum column width. But this one -static void write_n_ref_tables(struct reftable_stack *st, - size_t n) +static void write_n_ref_tables(struct reftable_stack *st, size_t n) is certainly an improvement. This one struct reftable_record rec = { .type = BLOCK_TYPE_REF, - .u = { - .ref = *ref - }, + .u = { .ref = *ref }, }; is hard to generalize but in this case it made a good suggestion. If we _expect_ that we will enumerate more members of .u, then the way originally written (plus trailing comma after the ".ref = *ref") would be easier to maintain. But since .u is a union, we won't be choosing more than one member to initialize by definition, so what was suggested by the check-style linter is certainly much better. Thanks.
