Github user HeartSaVioR commented on the pull request: https://github.com/apache/incubator-zeppelin/pull/581#issuecomment-168013136 Minor: How about extracting whole synchronized block to new private method? Actually setPropertyAndRestart() and restart() takes same logic except setting additional parameter and calling saveToFile(). setPropertyAndRestart() can call saveToFile() safely when new method is executed without InterpreterException. So new private method doesn't need to take care about safeToFile(). Next concern is that how we can unify logic whether two parameters are presented or not. It would be achieved easily with null check, but someone can think it's not beautiful. So I'd like to hear our opinions about this suggestion.
--- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---