Re: [GUILT PATCH 0/4] Add guards to guilt

From: Eric Lesh <eclesh@ucla.edu>
Date: Jul 30, 2007, 08:32 UTC
Message-ID: <873az6cmve.fsf@hubert.paunchy.net>
In-Reply-To: <20070730035443.GC22017@filer.fsl.cs.sunysb.edu>

Josef Sipek <jsipek@fsl.cs.sunysb.edu> writes:

Show 7 quoted lines

>
> I know I've been bad about forcing even myself to write new regression
> tests. Your patches modify things significantly enough, that I'd like to see
> some regression tests cases to make sure that user's data is not eaten
> (e.g., a bug in the guard setting code could blow away the series file =>
> very bad).
>

Yeah, I'll try and make one. 070-guards.sh to test guilt-guard and guilt-select plus pushing and popping?

There's also sed -i in a few places. For integrity purposes, are a cp and sed better?

Show 16 quoted lines

>> This introduces the command `get_guarded_series`, which just lists
>> patches that are to be applied based on the guards.  It also makes
>>         eidx=`wc -l < $applied`
>> inaccurate if you're using it as an index into get_series.
>
> The index-based patch finding is a bit nasty anyway.
>
>> If you change guards on a patch or select a different guard while
>> patches are applied, some commands might get confused. guilt pop -a will fix
>> everything though.  Usually, it's best to pop -a before fiddling with
>> guards anyway.
>
> Is this a problem with other projects' implementations of guards as well?
> Perhaps printing a warning if a new guard is set when there are applied
> patches would be in order?
>

Yeah, they have this problem too, but tell you so when you select, so guilt should too. I'll fix that up. Mercurial also has two options which do the popping and reapplying for you, which I'll try and implement also.

Thanks a lot for the review. Things were pretty ugly, but with your help it should look much better.

	Eric