Re: [Quilt-dev] [PATCH] quilt drop command

[Jean Delvare]

Hi Josh,

> The patch below adds a 'quilt drop' command.  This command basically
> something similar to a 'quilt pop', but without actually removing the
> applied patch from the modified files.  It also removes the patch name
> from the series file and cleans up the backup files in the .pc
> directory.  It should leave the actual patch file in the patches
> directory however, which is handy for those "oops, I screwed up"
> moments.
> (...)
> If there are no objections, please consider applying this patch.
> Otherwise, let me know and I'll fixup whatever needs fixing :).

Here are a few random comments on your "drop" patch.

1* There are still a number of references to the original "commit" name,
both in the comments before the actual patch, and in the patch itself.

2* The code of your "drop" command is so similar to the "pop" command
that I really wonder if it wouldn't make more sense to add a --drop
option to the "pop" command instead. The only real differences, as far
as I can see, are the flags passed to backup-files, and the extra call
to remove_from_series. Could you try implementing the "drop"
functionality as an option to the pop command, so that we can see which
approach is the best?

3* Indentation is broken in the command-line options handling part of
your code (lines 195-217).

4* You did not properly discard the -R option, there are still two
occurences of $opt_remove in your patch.

Note that I am not entirely convinced by the "drop" name. When I hear
"drop" I somehow think of it as a synonymous for "discard", which is the
exact opposite of what the command does. I expect it to confuse some
users as well if we keep it. Not that I have any obvious replacement
name (except for the random list I posted last week)... Maybe if it
becomes an option to the "pop" command it will bring new ideas for a
better name.

-- 
Jean Delvare