> On Jan. 28, 2015, 2:34 a.m., Eric Olander wrote:
> > core/src/main/scala/kafka/metrics/KafkaMetricsGroup.scala, line 66
> > <https://reviews.apache.org/r/30321/diff/1/?file=836389#file836389line66>
> >
> > val scope = tagsName.map {t => nameBuilder.append(",").append(t)}.orNull
>
> Eric Olander wrote:
> Sorry - I have a typo in the above comment
> val scope = tagsName.map {t => nameBuilder.append(",").append(t);
> t}.orNull
Alright - sorry to keep churning on this. Things are always clearer in the
morning. Really there are two things happening here that are being handled
though side effects - assignment of the scope var and updating nameBuilder.
I'd prefer to use less of the Java style and move more to the Scala way of
handling things like this. So,
val scope = tagsName.orNull
That will preserve the original semantics of what you did - scope is either the
contents of the Some or null.
tagsName.foreach {t => nameBuilder.append(",").append(t)}
Side-effecting code on collections is typically performed within a foreach so
this makes things more apparent.
- Eric
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30321/#review69940
-----------------------------------------------------------
On Jan. 27, 2015, 4:16 p.m., Jun Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30321/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2015, 4:16 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: kafka-1902
> https://issues.apache.org/jira/browse/kafka-1902
>
>
> Repository: kafka
>
>
> Description
> -------
>
> fix metric name by adding tags as scope
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/metrics/KafkaMetricsGroup.scala
> e9e49180f6de45f98e79374f519f6097b4fc8637
>
> Diff: https://reviews.apache.org/r/30321/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jun Rao
>
>
