Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12195 )
Change subject: generic_iterators: move MergeIterState into the header ...................................................................... Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/12195/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12195/2//COMMIT_MSG@10 PS2, Line 10: forward declare MergeIterState anymore. another option here to consider is moving the definition of UnionIterator and MergeIterator into the .cc file, and then only expose factory methods (which return the super-interface type instead of the underlying impl types). Would that work or do callers need the specific impl types? (not a big deal, just wondering if it would be just as easy and improve compilation time and encapsulation) -- To view, visit http://gerrit.cloudera.org:8080/12195 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00f3ec1a0007ab1f554f4b799884e94c90c7cc9b Gerrit-Change-Number: 12195 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Wed, 09 Jan 2019 20:57:21 +0000 Gerrit-HasComments: Yes
