Patchwork D7720: rust-discovery: moved some methods to the wrapper enum

login
register
mail settings
Submitter phabricator
Date Dec. 24, 2019, 1:50 p.m.
Message ID <differential-rev-PHID-DREV-wbdy5qedorywpbevjxrc-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/44041/
State New
Headers show

Comments

phabricator - Dec. 24, 2019, 1:50 p.m.
gracinet created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  In the primary switch to the typestate pattern, these
  methods have been kept on the `WithUndecided` struct, that
  had most of the original methods of `PartialDiscovery`, so
  that the change would be readable.
  
  But we feel it makes more sense to have them on the wrapper
  enum.
  
  `common_heads` would also be a candidate for a last stage method,
  we wouldn't need that lenghty warning docstring that it's not
  relevant if `is-complete` is `true`

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7720

AFFECTED FILES
  rust/hg-core/src/discovery.rs

CHANGE DETAILS




To: gracinet, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel

Patch

diff --git a/rust/hg-core/src/discovery.rs b/rust/hg-core/src/discovery.rs
--- a/rust/hg-core/src/discovery.rs
+++ b/rust/hg-core/src/discovery.rs
@@ -198,19 +198,23 @@ 
         ))
     }
 
+    fn get_common(&self) -> &MissingAncestors<G> {
+        match self {
+            Com(c) => &c.common,
+            Und(u) => &u.common,
+        }
+    }
+
     /// Do we have any information about the peer?
     pub fn has_info(&self) -> bool {
-        match self {
-            Com(c) => c.common.has_bases(),
-            Und(u) => u.has_info(),
-        }
+        self.get_common().has_bases()
     }
 
     /// Did we acquire full knowledge of our Revisions that the peer has?
     pub fn is_complete(&self) -> bool {
         match self {
             Com(_) => false,
-            Und(u) => u.is_complete(),
+            Und(u) => u.undecided.is_empty(),
         }
     }
 
@@ -226,10 +230,7 @@ 
     /// would be more appropriate for normal Rust callers, dropping `self`
     /// if it is complete.
     pub fn common_heads(&self) -> Result<HashSet<Revision>, GraphError> {
-        match self {
-            Com(c) => c.common.bases_heads(),
-            Und(u) => u.common_heads(),
-        }
+        self.get_common().bases_heads()
     }
 
     /// Provide statistics about the current state of the discovery process
@@ -479,31 +480,6 @@ 
         Ok(())
     }
 
-    /// Do we have any information about the peer?
-    pub fn has_info(&self) -> bool {
-        self.common.has_bases()
-    }
-
-    /// Did we acquire full knowledge of our Revisions that the peer has?
-    pub fn is_complete(&self) -> bool {
-        self.undecided.is_empty()
-    }
-
-    /// Return the heads of the currently known common set of revisions.
-    ///
-    /// If the discovery process is not complete (see `is_complete()`), the
-    /// caller must be aware that this is an intermediate state.
-    ///
-    /// On the other hand, if it is complete, then this is currently
-    /// the only way to retrieve the end results of the discovery process.
-    ///
-    /// We may introduce in the future an `into_common_heads` call that
-    /// would be more appropriate for normal Rust callers, dropping `self`
-    /// if it is complete.
-    pub fn common_heads(&self) -> Result<HashSet<Revision>, GraphError> {
-        self.common.bases_heads()
-    }
-
     fn compute_children_cache(
         graph: &G,
         undecided: impl Iterator<Item = Revision>,