Patchwork [1,of,4,V2,mergedriver] mergestate: add a constructor that sets up a fresh merge state

login
register
mail settings
Submitter Siddharth Agarwal
Date Nov. 18, 2015, 1:13 a.m.
Message ID <681b710bd541703659ae.1447809203@dev666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11452/
State Accepted
Headers show

Comments

Siddharth Agarwal - Nov. 18, 2015, 1:13 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1447808454 28800
#      Tue Nov 17 17:00:54 2015 -0800
# Node ID 681b710bd541703659aec3ac3c9a5b7b0d6e9493
# Parent  a879917dec18a4621476c8af18724f5d5696c663
mergestate: add a constructor that sets up a fresh merge state

Eventually, we'll move the read call out of the constructor. This will:
- avoid unnecessary reads when we're going to nuke the merge state anyway
- avoid raising an exception if there's an unsupported merge record

'clean' seems like a good name for it because I wanted to avoid anything with
the word 'new' in it, and 'reset' is more an action performed on a merge state
than a way to get a new merge state.

Thanks to Martin von Zweigbergk for feedback about naming this.
timeless - Nov. 18, 2015, 2:30 p.m.
I know this is on clowncopter, but do you really want the first line of the
commit message to use "fresh" instead of "clean"?
On Nov 17, 2015 8:14 PM, "Siddharth Agarwal" <sid0@fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1447808454 28800
> #      Tue Nov 17 17:00:54 2015 -0800
> # Node ID 681b710bd541703659aec3ac3c9a5b7b0d6e9493
> # Parent  a879917dec18a4621476c8af18724f5d5696c663
> mergestate: add a constructor that sets up a fresh merge state
>
> Eventually, we'll move the read call out of the constructor. This will:
> - avoid unnecessary reads when we're going to nuke the merge state anyway
> - avoid raising an exception if there's an unsupported merge record
>
> 'clean' seems like a good name for it because I wanted to avoid anything
> with
> the word 'new' in it, and 'reset' is more an action performed on a merge
> state
> than a way to get a new merge state.
>
> Thanks to Martin von Zweigbergk for feedback about naming this.
>
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -75,6 +75,14 @@ class mergestate(object):
>      statepathv1 = 'merge/state'
>      statepathv2 = 'merge/state2'
>
> +    @staticmethod
> +    def clean(repo, node=None, other=None):
> +        """Initialize a brand new merge state, removing any existing
> state on
> +        disk."""
> +        ms = mergestate(repo)
> +        ms.reset(node, other)
> +        return ms
> +
>      def __init__(self, repo):
>          self._repo = repo
>          self._dirty = False
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
Martin von Zweigbergk - Nov. 18, 2015, 5:04 p.m.
On Wed, Nov 18, 2015 at 6:33 AM timeless <timeless@gmail.com> wrote:

> I know this is on clowncopter, but do you really want the first line of
> the commit message to use "fresh" instead of "clean"?
>
Thanks, good catch. Will update on the clowncopter.


> On Nov 17, 2015 8:14 PM, "Siddharth Agarwal" <sid0@fb.com> wrote:
>
>> # HG changeset patch
>> # User Siddharth Agarwal <sid0@fb.com>
>> # Date 1447808454 28800
>> #      Tue Nov 17 17:00:54 2015 -0800
>> # Node ID 681b710bd541703659aec3ac3c9a5b7b0d6e9493
>> # Parent  a879917dec18a4621476c8af18724f5d5696c663
>> mergestate: add a constructor that sets up a fresh merge state
>>
>> Eventually, we'll move the read call out of the constructor. This will:
>> - avoid unnecessary reads when we're going to nuke the merge state anyway
>> - avoid raising an exception if there's an unsupported merge record
>>
>> 'clean' seems like a good name for it because I wanted to avoid anything
>> with
>> the word 'new' in it, and 'reset' is more an action performed on a merge
>> state
>> than a way to get a new merge state.
>>
>> Thanks to Martin von Zweigbergk for feedback about naming this.
>>
>> diff --git a/mercurial/merge.py b/mercurial/merge.py
>> --- a/mercurial/merge.py
>> +++ b/mercurial/merge.py
>> @@ -75,6 +75,14 @@ class mergestate(object):
>>      statepathv1 = 'merge/state'
>>      statepathv2 = 'merge/state2'
>>
>> +    @staticmethod
>> +    def clean(repo, node=None, other=None):
>> +        """Initialize a brand new merge state, removing any existing
>> state on
>> +        disk."""
>> +        ms = mergestate(repo)
>> +        ms.reset(node, other)
>> +        return ms
>> +
>>      def __init__(self, repo):
>>          self._repo = repo
>>          self._dirty = False
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> https://selenic.com/mailman/listinfo/mercurial-devel
>>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
Siddharth Agarwal - Nov. 18, 2015, 5:53 p.m.
On 11/18/15 06:30, timeless wrote:
>
> I know this is on clowncopter, but do you really want the first line 
> of the commit message to use "fresh" instead of "clean"?
>

I don't care very much either way, but if Martin does then sure.


> On Nov 17, 2015 8:14 PM, "Siddharth Agarwal" <sid0@fb.com 
> <mailto:sid0@fb.com>> wrote:
>
>     # HG changeset patch
>     # User Siddharth Agarwal <sid0@fb.com <mailto:sid0@fb.com>>
>     # Date 1447808454 28800
>     #      Tue Nov 17 17:00:54 2015 -0800
>     # Node ID 681b710bd541703659aec3ac3c9a5b7b0d6e9493
>     # Parent  a879917dec18a4621476c8af18724f5d5696c663
>     mergestate: add a constructor that sets up a fresh merge state
>
>     Eventually, we'll move the read call out of the constructor. This
>     will:
>     - avoid unnecessary reads when we're going to nuke the merge state
>     anyway
>     - avoid raising an exception if there's an unsupported merge record
>
>     'clean' seems like a good name for it because I wanted to avoid
>     anything with
>     the word 'new' in it, and 'reset' is more an action performed on a
>     merge state
>     than a way to get a new merge state.
>
>     Thanks to Martin von Zweigbergk for feedback about naming this.
>
>     diff --git a/mercurial/merge.py b/mercurial/merge.py
>     --- a/mercurial/merge.py
>     +++ b/mercurial/merge.py
>     @@ -75,6 +75,14 @@ class mergestate(object):
>          statepathv1 = 'merge/state'
>          statepathv2 = 'merge/state2'
>
>     +    @staticmethod
>     +    def clean(repo, node=None, other=None):
>     +        """Initialize a brand new merge state, removing any
>     existing state on
>     +        disk."""
>     +        ms = mergestate(repo)
>     +        ms.reset(node, other)
>     +        return ms
>     +
>          def __init__(self, repo):
>              self._repo = repo
>              self._dirty = False
>     _______________________________________________
>     Mercurial-devel mailing list
>     Mercurial-devel@selenic.com <mailto:Mercurial-devel@selenic.com>
>     https://selenic.com/mailman/listinfo/mercurial-devel
>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -75,6 +75,14 @@  class mergestate(object):
     statepathv1 = 'merge/state'
     statepathv2 = 'merge/state2'
 
+    @staticmethod
+    def clean(repo, node=None, other=None):
+        """Initialize a brand new merge state, removing any existing state on
+        disk."""
+        ms = mergestate(repo)
+        ms.reset(node, other)
+        return ms
+
     def __init__(self, repo):
         self._repo = repo
         self._dirty = False