[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Monotone-devel] resolving name conflicts; code style
From: |
Zack Weinberg |
Subject: |
Re: [Monotone-devel] resolving name conflicts; code style |
Date: |
Fri, 9 May 2008 12:24:17 -0400 |
On Fri, May 9, 2008 at 4:54 AM, Stephen Leake
<address@hidden> wrote:
> I've passed app.opts down several layers to merge.cc
> resolve_conflicts. That function then checks for the presence of
> --resolve_conflicts or --resolve_conflicts_file, and calls functions
> that parse the string or file to resolve conflicts.
>
> Is passing app.opts down that low ok? I didn't see a better way to do
> it.
I would suggest passing just the relevant fields of app.opts. This is both
more maintainable (because it's obvious which options can affect the behavior)
and more convenient if one ever wants to add another call site that specifies
one of the options itself.
Tangentially, please spell multiword options with dashes between words
instead of underscores. (--resolve-conflicts, --resolve-conflicts-file). It's
easier to type that way.
> In merge.hh and a couple of other files, I added a copyright line for
> myself. What is the policy on copyright?
I'm not aware of any specific policy.
> In roster_merge.cc, I have this for representing the various
> resolution options:
>
> enum resolution_t {resolved_none, resolved_content_ws, resolved_rename};
>
> string image (resolution_t resolution)
> {
> switch (resolution)
> {
> case resolved_none:
> return "resolved_none";
>
> case resolved_content_ws:
> return "resolved_content_ws";
>
> case resolved_rename:
> return "resolved_rename";
> }
>
> return ""; // suppress bogus compiler warning
> }
For this sort of thing I would actually use bare char * to avoid
constructor overhead, but I don't insist on it. symbol is probably
better from a type system perspective.
For the switch statement, I would recommend a default: clause
that's an invariant failure, like so:
switch (thing)
{
case one: ...; break;
case two: ...; break;
case three: ...; break;
...
default:
I(false);
}
and don't put anything after the switch.
zw
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, (continued)
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, Markus Schiltknecht, 2008/05/08
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, Thomas Moschny, 2008/05/08
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, Markus Schiltknecht, 2008/05/08
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, William Uther, 2008/05/08
- [Monotone-devel] File resurrection, William Uther, 2008/05/08
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, Stephen Leake, 2008/05/09
- Re: [Monotone-devel] resolving name conflicts; code style, Stephen Leake, 2008/05/09
- Re: [Monotone-devel] resolving name conflicts; code style, Stephen Leake, 2008/05/09
- Re: [Monotone-devel] resolving name conflicts; code style,
Zack Weinberg <=
- Re: [Monotone-devel] resolving name conflicts; code style, Stephen Leake, 2008/05/10
- Re: [Monotone-devel] resolving name conflicts; code style, Zack Weinberg, 2008/05/10
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, Markus Schiltknecht, 2008/05/09
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, Stephen Leake, 2008/05/09
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, William Uther, 2008/05/09
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, Markus Schiltknecht, 2008/05/09
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, Derek Scherger, 2008/05/09
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, Thomas Moschny, 2008/05/10
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, Justin Patrin, 2008/05/06
- Re: [Monotone-devel] resolving name conflicts; file suturing vs drop, Markus Schiltknecht, 2008/05/07