> On July 10, 2015, 4:46 p.m., Gwen Shapira wrote: > > core/src/main/scala/kafka/admin/TopicCommand.scala, lines 72-73 > > <https://reviews.apache.org/r/34403/diff/4/?file=1008271#file1008271line72> > > > > This is a bit unclean. I think its more idiomatic when the catch block > > includes the System.exit(1). > > > > Also, I'm afraid that printing the entire stack trace is intimidating > > to non-developers who use the CLI. Perhaps the stack trace should go under > > log.error(...)?
Calling System.exit(1) in catch block results unexecuted finally block. http://stackoverflow.com/questions/1410951/how-does-javas-system-exit-work-with-try-catch-finally-blocks http://stackoverflow.com/questions/15264076/regarding-excuting-finally-block-in-system-exit-case-also-by-adding-shutdownhook?lq=1 log.error() used for printing stackTrace. - Manikumar Reddy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34403/#review91310 ----------------------------------------------------------- On July 10, 2015, 5:44 p.m., Manikumar Reddy O wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34403/ > ----------------------------------------------------------- > > (Updated July 10, 2015, 5:44 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2198 > https://issues.apache.org/jira/browse/KAFKA-2198 > > > Repository: kafka > > > Description > ------- > > Addressing Gwen's comments > > > Diffs > ----- > > core/src/main/scala/kafka/admin/TopicCommand.scala > a2ecb9620d647bf8f957a1f00f52896438e804a7 > > Diff: https://reviews.apache.org/r/34403/diff/ > > > Testing > ------- > > > Thanks, > > Manikumar Reddy O > >