kevincox accepted this revision. kevincox added inline comments. INLINE COMMENTS
> ancestors.rs:120 > + /// This is mostly meant for iterators backing a lazy ancestors set > + pub fn empty(&self) -> bool { > + if self.visit.len() > 0 { Can you clarify this `is_empty` to match rust convention? > ancestors.rs:121 > + pub fn empty(&self) -> bool { > + if self.visit.len() > 0 { > + return false; `if !self.visit.is_empty()` > ancestors.rs:124 > + } > + let seen = self.seen.len(); > + if seen > 1 { I think this variable makes the code harder to read. I would just repeat the calls to `.len()`. > ancestors.rs:196 > + > + #[inline] > + pub fn contains(&mut self, rev: Revision) -> Result<bool, GraphError> { In general I wouldn't add these without careful benchmarking. In this case the compiler can trivially notice that this method is a good inlining candidate. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5440 To: gracinet, #hg-reviewers, kevincox Cc: durin42, kevincox, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel