[PATCH] status: add option to show status of files in subrepos

Nicolas Dumazet nicdumz at gmail.com
Tue Jun 8 05:13:02 CDT 2010


Hello David, and welcome aboard!

2010/6/7 David Mitchell <david.mitchell at telogis.com>:
> # HG changeset patch
> # User David Mitchell <david.mitchell at telogis.com>
> # Date 1275870701 -43200
> # Branch stable
> # Node ID a26fd6d9d9f60da713f64a92f5713d724c94adef
> # Parent  d3ebb1a0bc49559e1e41d37f69c2afa06722563e
> Add option to status to see changes in all subrepos.
> Only used by status command so it has no impact on the way commits are done,
> and therefore no effect on the default commit status comment.
> Doesn't show which changes are files in subrepos and which are the top level
> repo.

Thanks for the inline (patchbombed) patch. That's the proper way to
ask for review here!

I like the idea, but I will however not look too much at the code
because... it is not tested =)
If you want people to review seriously the patch, you might want to
resend a version with a few tests for the new feature.

Because it changes command semantics, it will also break a few tests,
and in particular test-debugcomplete. It's okay with us, but you will
need to submit along the changes to the output. (easy trick: "python
run-tests.py -i test-debugcomplete" will run interactively the test
and ask you if changes are the ones you want, before updating for you
the expected output.) In general, running the full test suite (make
tests) before sending in patches do help =)

As for the log message, we usually use the following convention:
"""
component: very short description

longer description after a linebreak. This is a long description
and it is wrapped at 80 chars.
"""
Here for example, we'd expect "status: add --subrepo option to list
changes in subrepos\n\netc..."

> diff -r d3ebb1a0bc49 -r a26fd6d9d9f6 mercurial/commands.py
[...]
>      def status(self, node1='.', node2=None, match=None,
> -               ignored=False, clean=False, unknown=False):
> +               ignored=False, clean=False, unknown=False, showSubs=False):
>          """return status of files between two nodes or node and working
> directory

We are not too fond of camelCase here ( have a peek at
http://mercurial.selenic.com/wiki/BasicCodingStyle and at the more
generic http://mercurial.selenic.com/wiki/ContributingChanges
explaining why we get so picky when reviewing patches, and what one
should know to get through more easily :p ).
"subrepo" might be enough, or "showsubrepo" if you feel that you need
to precise more the semantics.

Regards,

-- 
Nicolas Dumazet — NicDumZ


More information about the Mercurial-devel mailing list