dsmiley commented on code in PR #1627: URL: https://github.com/apache/solr/pull/1627#discussion_r1223844855
########## solr/prometheus-exporter/src/java/org/apache/solr/prometheus/collector/MetricSamples.java: ########## @@ -71,4 +83,8 @@ public List<Collector.MetricFamilySamples> asList() { .filter(value -> !value.samples.isEmpty()) .collect(Collectors.toList()); } + + private void addUnseenSamples(Collector.MetricFamilySamples metricFamilySamples) { + seenSamples.addAll(metricFamilySamples.samples); Review Comment: This is confusing to me... a method named addUnseenSamples that calls seenSamples.addAll. It seems inverted (and seemed as such at the call sites too; I wonder how does one add something you don't see?), yet I can also imagine how one might justify its name. Personally I think inlining this method would be better and would resolve it. One less thing to name :-) I know this is a philosophical bike shed for developers; some people love one-liner methods -- admittedly I'm not one of those people. If you love it as is, okay. Maybe its just me. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org