[ 
https://issues.apache.org/jira/browse/CASSANDRA-16188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17378768#comment-17378768
 ] 

Aleksei Zotov commented on CASSANDRA-16188:
-------------------------------------------

[~sumanth.pasupuleti]

I checked the changes, in general they look good to me. However, I have two 
nits:
 # I'd add a static import for {{Assert}}:
{code}
Assert.assertEquals(0, (long) keyspaceMetrics.memtableColumnsCount.getValue());
==>
assertEquals(0, (long) keyspaceMetrics.memtableColumnsCount.getValue());
{code}
I feel "Assert." prefix just pollutes code and makes reading harder.
 # I'd use {{assertThat.isGreaterThan}} instead of a synthetic way to do the 
same via {{assertTrue}}: 
{code}
assertTrue(keyspaceMetrics.memtableColumnsCount.getValue() > 0);
==>
assertThat(keyspaceMetrics.memtableColumnsCount.getValue()).isGreaterThan(0)
{code}
I feel {{isGreaterThan}} looks clearer and more emphatic.

Anyway, both comment are purely stylistic, so it is up to you whether to 
address them or not. 

PS:
I'm not a project committer, just trying to help in reviewing changes as per 
Benjamin's email.

> Add more tests to cover Keyspace and Table metrics 
> ---------------------------------------------------
>
>                 Key: CASSANDRA-16188
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-16188
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Test/dtest/java, Test/unit
>            Reporter: Benjamin Lerer
>            Assignee: Sumanth Pasupuleti
>            Priority: Normal
>             Fix For: 4.0.x
>
>
> Several Keyspace and Table related metrics are currently not tested.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to