That makes perfect sense. Why lose the refactor over a tiny logic change if we have to revert etc. Thanks!
On Fri, Jan 25, 2013 at 10:39 PM, Nitay Joffe (JIRA) <[email protected]>wrote: > > [ > https://issues.apache.org/jira/browse/GIRAPH-469?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13563387#comment-13563387] > > Nitay Joffe commented on GIRAPH-469: > ------------------------------------ > > I looked it over again. +1 still good. > In the future please keep refactors and logic change separate, it makes > things much easier to review and really comes in handy if we ever have to > go back and look through log, revert, etc. > > > Cleanup GraphMapper > > ------------------- > > > > Key: GIRAPH-469 > > URL: https://issues.apache.org/jira/browse/GIRAPH-469 > > Project: Giraph > > Issue Type: Improvement > > Reporter: Nitay Joffe > > Assignee: Eli Reisman > > Attachments: GIRAPH-469-1-eli-idea.patch, GIRAPH-469-2.patch, > GIRAPH-469-3.patch, GIRAPH-469-4.patch, GIRAPH-469-5.patch > > > > > > I don't see why we even call a map() method seeing as we are overriding > run(). We are clearly not particularly "mapreduce-y" so we should make it > our entry point more clear than a map(). Also I think we should have > something like a WorkerThread similar to MasterThread and clean up all of > this to just creare whichever threads the node is assigned roles of. > > Link to review board: > > https://reviews.apache.org/r/8898/ > > -- > This message is automatically generated by JIRA. > If you think it was sent incorrectly, please contact your JIRA > administrators > For more information on JIRA, see: http://www.atlassian.com/software/jira >
