search

instaclustr / cassandra-exporter

Java agent for exporting Cassandra metrics to Prometheus
Apache License 2.0
73 stars 46 forks source link

Refactor HarvesterOptionsTest to use assertj #81

Open eperott opened 4 years ago

eperott commented 4 years ago

Also, moving test class into the correct module.

eperott commented 4 years ago

Proposing to make test asserts with assertJ style, as it generally improves readability. Of course, this is a matter of opinion/preference.

And speaking of preferences, should we use Test prefix or suffix in test class names in cassandra-exporter. I'm proposing to use TestMyClass (over MyClassTest). In my experience this will save me a couple of key strokes every time I'm looking up a class in IntelliJ (searching with ctrl+n).

Lot's of opinions here. :-) Let me know what you think.

smiklosovic commented 4 years ago

I prefer MyClassTest but if we want to change it I dont mind however we should be consistent so we should rename all of them. MyClassTest is more readable in my books as I just do not need to parse in my head what test it is after that Test prefix but only my preferencies speak here ...

eperott commented 4 years ago

Agree, being consistent is the most important thing. Which is why I brought it up - currently we're not.

Whether we go for prefix or suffix is not something we need to debate at any length. You guys, being maintainers, should probably just decide which way to go.