sijie commented on issue #570: Multiple active entrylogs URL: https://github.com/apache/bookkeeper/issues/570#issuecomment-368826780 > What is the rush? I am not suggesting a rush. What I am suggesting here is to respect to what have been discussed and agreed on. What I am saying here is that this change has been raised up and discussed a couple of time since a few months ago, and we have sort of settled down about the approach and the implementation we need to take. It doesn't make any sense for re-doing all this discussion and wasting the time we had in the discussion. That's what I am suggesting for not changing the direction on how things have been implemented here, otherwise that means JV, Charan and me have to go through another round of discussion and Charan might have to redo all his changes, which doesn't seems to be worth doing that. Also if you take a closer look at Charan's change, it isn't really too much different from what you suggested in your recent comment. It is just a matter where does the abstraction happen at. His change is at the entry log manager level, which he attempts to provide a entry log manager to abstract how different implementations manage entry log files. what you suggest might be similar but slight different abstraction interface, which from my personal view, there is no real fundamental differences. That's why I suggest that if he can improve his current `EntryLogManager` interface and hide the details on how different implementations managing entry log files. That will be a good implementation to ship this feature. > I've seen this suggestion to just wave through a patch for expedience a couple of times now, and I think it's a terrible practice. I remember wanting to do a refactor once the twitter changes got in. That never happened for a number of reason. Then we discussed a refactor of checkpointing once yahoo changes are in. I think people would have different views on things, such as how to move forward the project, how to implement a feature, how is the code organized. We are in a public project, collaborating in the same project, different organizations might also have different priorities on things. For example, for us, "per entry log" might not be priority, while it might be a priority for salesforce. That is something that we should have to respect to each other. The examples you described here about twitter and yahoo cases also came from the priorities that twitter and yahoo used to have, whether those refactor are needed are also have to based on priorities that each organization has. I am not sure that would be the thing called "terrible practice". from my personal experiences, "shipit" is never a terrible thing to do, and that's how different organizations can really move the project into production and shipit on production and move it forward, rather than being stuck at waiting perfect refactor (which I don't think there is going to be a perfect code refactor). so in my view, "shipit" is much important for a lot of organizations than other things. The tech debts you are mentioning about really only come to true when there are priorities to move them forward. > This patch in itself is 2900 LOC. Unless some takes two whole days to review that, then its not going to get a thorough review before submission. This problem compounds each time the patch is updated. That alone is reason to slow down. That is a real issue for any changes and for any person reviewing the change. I don't think things are going to change if changing a different approach to implement this. You will probably feel it easy to review, because the implementation is aligned to what you have been thinking in your mind. But the situation might still remain true to other reviewers reviewing the change. I have found it useful if I think from the author's perspective and understand what he was thinking, that would be easier for me to understand his change and know how different people think of the same problem and adjust to coding style according, that would make reviews easier and people can come to agreement much faster.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
