[PATCH 1 of 2] rebase: added comments

[Pierre-Yves David]

On 10/13/2015 03:07 PM, Christian Delahousse wrote:
> # HG changeset patch
> # User Christian Delahousse <cdelahousse at fb.com>
> # Date 1444771035 25200
> #      Tue Oct 13 14:17:15 2015 -0700
> # Node ID 4a52314eb7a983d2d0adbaadbe4d122132455e2a
> # Parent  a38924f7680c6b7d95e14ade999c35748c9dcafd
> rebase: added comments

I'm happy to take the second block but remains unconvinced by the first one.

> Added comments describing the state variable and constants used throughout the
> rebase extension

(missing .)

>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -23,9 +23,14 @@
>   from mercurial.i18n import _
>   import os, errno
>
> +# The following constants are used throughout the rebase module. The ordering of
> +# their values must be maintained.
> +
> +# Indicates that a revision needs to be rebased
>   revtodo = -1
>   nullmerge = -2
>   revignored = -3
> +# To do with obselescence
>   revprecursor = -4

- Why about the 2 others ? Documenting these constants is a good idea 
but why stop halfway?

- You misspelled obsolescence.

- I'm do not get what this last comment about revprecursor is try to say.



>
>   cmdtable = {}
> @@ -187,6 +192,9 @@
>       originalwd = target = None
>       activebookmark = None
>       external = nullrev
> +    # Mapping between thes old revision id and either what is the new rebased
> +    # revision or what needs to be done with the old revsion. The state dict
> +    # will be what contains most of the rebase progress state.
>       state = {}
>       skipped = set()
>       targetancestors = set()
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>

-- 
Pierre-Yves David