Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> + if (progress) {
> + strvec_push(&extra_args, "-v");
> + strvec_push(&extra_args, "--progress-title");
> + strvec_push(&extra_args, _("Unbundling objects"));
Nice. I would have expected to see pushl() though.
> + }
> +
> + ret = !!unbundle(the_repository, &header, bundle_fd, progress ?
> + &extra_args : NULL) ||
Again, I wouldn't make the &extra_args conditional to progress
here. Future code change may decide to pass more args to underlying
index-pack and the criteria for doing so may be different from
progress.
If this code cares about readability, it should uncondtionally pass
&extra_args.
If this code cares about readability *and* micro-optimization, then
the condition should be on !!extra_args.nr, not on whatever set of
conditions happen to be used in today's code to throw items into
extra_args array.
Other than that, this was a pleasant read.
Thanks.
