[PATCH 2 of 5] test-revert: don't unnecessarily pipe through 'cat'

[Pierre-Yves David]

On 11/02/2014 11:05 PM, Martin von Zweigbergk wrote:
>
>
> On Sun Nov 02 2014 at 2:45:40 PM Pierre-Yves David
> <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>>
> wrote:
>
>
>
>     On 11/02/2014 10:19 PM, Martin von Zweigbergk wrote:
>      > # HG changeset patch
>      > # User Martin von Zweigbergk <martinvonz at google.com
>     <mailto:martinvonz at google.com>>
>      > # Date 1413682221 25200
>      > #      Sat Oct 18 18:30:21 2014 -0700
>      > # Branch stable
>      > # Node ID 9439327a222474b6af32ed9f717bfc__e81f90e53b
>      > # Parent  5df55ad6918a76870695dfe8551683__09bb5da27c
>      > test-revert: don't unnecessarily pipe through 'cat'
>      >
>      > diff --git a/tests/test-revert.t b/tests/test-revert.t
>      > --- a/tests/test-revert.t
>      > +++ b/tests/test-revert.t
>      > @@ -730,7 +730,7 @@
>      >
>      >   Setup working directory
>      >
>      > -  $ python ../gen-revert-cases.py wc | cat
>      > +  $ python ../gen-revert-cases.py wc
>      >     $ hg addremove --similarity 0
>      >     removing added_removed
>      >     removing added_revert
>
>
>     check-code says hi:
>
>          Skipping mercurial/httpclient/__socketutil.py it has
>     no-che?k-code (glob)
>     +  tests/test-revert.t:733:
>     +   >   $ python ../gen-revert-cases.py wc
>     +   filter wc output
>     +  [1]
>
>
> Oops, sorry about that. What's the purpose of it? The check was in the
> first version of check-code, so I couldn't find any explanation for it
> in the changelog.

I don't known why it is here. We should probably ask Matt about it (cced 
him)

>     I'm not sure about the 3 others patches. What is wrong with some
>     duplicated case? Why is it worth introducing special casing to
>     remove them ?
>
>
> I think the file histories are clearer when expressed in terms of the
> states rather than transitions, so in the end (after ~7 more patches),
> e.g. 'modified_untracked-clean' becomes
> 'content1_content2_untracked_content2'. The generation script actually
> becomes a bit simpler at that point. I suppose a 11-patch series is too
> long. Do you want to see the end state in some other form before you decide?

You should include this kind of high level goal in your commit message:

   I'm doing XXX because we want to be able to do YYY in the future. The 
change is correct because YYY

This seems a reasonable changes, but it makes is a bit (but just a bit) 
harder to compute the expected revert action from the name (since you 
have to make the diff yourself).

I'm curious about the whole series. Can you push to non publishing repo 
for me to have a look ?

(note, your naming scheme should make a clearer distinction between each 
part, eg: content1_content2_untracked-content2)


-- 
Pierre-Yves David

PS: not sure what you use as a MUA but is appears to be quite bad at 
standard quoting.