> On Feb. 13, 2015, 6:24 p.m., Eric Olander wrote: > > core/src/main/scala/kafka/server/KafkaApis.scala, line 303 > > <https://reviews.apache.org/r/29912/diff/3/?file=862697#file862697line303> > > > > This code could be done using map and getOrElse on the Option rather > > than using pattern matching/reflection. > > > > logManager.getLog(topicAndPartition).map{log => fetchOffsetsBefore(log, > > timestamp, maxNumOffsets) > > }.getOrElse { > > if (timestamp == OffsetRequest.LatestTime || timestamp == > > OffsetRequest.EarliestTime) > > Seq(0L) > > else > > Nil > > } > > Sriharsha Chintalapani wrote: > Eric, > Thanks for the review. But the above code is not added as part of > this patch and also not releated to this JIRA so don't want to change it as > part of this patch. > > Gwen Shapira wrote: > Eric, > > I noticed you have a lot of good comments on code style when reviewing > patches. I think we are all happy to improve code we are touching as part of > the patch, but we are also trying to keep the scope of patches mostly > contained and not touch unrelated code. This is so if we accidentally break > something it will be easier to figure out what happened. > > Feel free to open new JIRA for those unrelated issues and submit patches. > > Gwen
This is why I'm not marking these as issues in the review - they're just comments. I'm hoping to plant the seed in peoples minds to think of Option not in terms of Some and None, but instead to use the higher level APIs that Option provides. I'm perfectly willing to open Jiras and file patches. - Eric ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29912/#review72405 ----------------------------------------------------------- On Feb. 13, 2015, 12:46 a.m., Sriharsha Chintalapani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29912/ > ----------------------------------------------------------- > > (Updated Feb. 13, 2015, 12:46 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1852 > https://issues.apache.org/jira/browse/KAFKA-1852 > > > Repository: kafka > > > Description > ------- > > KAFKA-1852. OffsetCommitRequest can commit offset on unknown topic. Added > contains method to MetadataCache. > > > Diffs > ----- > > core/src/main/scala/kafka/server/KafkaApis.scala > 6ee7d8819a9ef923f3a65c865a0a3d8ded8845f0 > core/src/main/scala/kafka/server/MetadataCache.scala > 4c70aa7e0157b85de5e24736ebf487239c4571d0 > core/src/main/scala/kafka/server/OffsetManager.scala > 83d52643028c5628057dc0aa29819becfda61fdb > core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala > 5b93239cdc26b5be7696f4e7863adb9fbe5f0ed5 > > Diff: https://reviews.apache.org/r/29912/diff/ > > > Testing > ------- > > > Thanks, > > Sriharsha Chintalapani > >
