On Sat, Oct 5, 2019 at 3:44 PM Elijah Newren <newren@xxxxxxxxx> wrote:
>
> On Thu, Sep 19, 2019 at 3:07 PM Derrick Stolee via GitGitGadget
> <gitgitgadget@xxxxxxxxx> wrote:
> > +static int write_patterns_and_update(struct pattern_list *pl)
> > +{
> > + char *sparse_filename;
> > + FILE *fp;
> > +
> > + sparse_filename = get_sparse_checkout_filename();
> > + fp = fopen(sparse_filename, "w");
> > + write_patterns_to_file(fp, pl);
> > + fclose(fp);
> > + free(sparse_filename);
> > +
> > + clear_pattern_list(pl);
>
> It seems slightly odd that pl is passed in but cleared in this
> function rather than in the caller that created pl. Should this be
> moved to the caller, or, alternatively, a comment added to explain
> this side-effect for future callers of the function?
>
> The rest of the patch looked good to me.
Actually, thought of something else. What if the user calls 'git
sparse-checkout set ...' without first calling 'git sparse-checkout
init'? Should that report an error to the user, a suggestion to
follow it up with 'sparse-checkout init', or should it just call
sc_set_config() behind the scenes and allow bypassing the init
subcommand?
