Re: [PATCH v2 04/11] sparse-checkout: 'set' subcommand

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/5/2019 8:30 PM, Elijah Newren wrote:
> 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?

Maybe a warning would suffice. I still think the workflow of the
following is most correct, and not difficult to recommend:

* "git sparse-checkout init [--cone]" -OR- "git clone --sparse"
* git sparse-checkout set [stuff]
* git sparse-checkout disable

Thanks,
-Stolee



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux