influxdata / kapacitor

Open source framework for processing, monitoring, and alerting on time series data
MIT License
2.32k stars 492 forks source link

tickfmt readability changes #937

Open mattyjones opened 8 years ago

mattyjones commented 8 years ago

I really like the use of tickfmt but I has some questions as to why specific conventions were implemented and if a PR to adjust them would be accepted.

This looks ugly to me, I conceede it is a matter of personal taste but in large tick scripts I would like to see something a little more condensed.

// Datastream
// Define the data that will be acted upon
var db = 'telegraf'

var group = 'host'

var metricType = 'cpu'

var metric = 'usage_steal'

var metricFilter = 'cpu-total'

var rPolicy = 'default'

This preserves some of the whitespace but the variables are not spaced out with a newline. IMHO this not only saves additional scrolling of the code but presents the code in a more readable manner.

// Datastream
// Define the data that will be acted upon

var db            = 'telegraf'
var group         = 'host'
var metricType    = 'cpu'
var metric        = 'usage_steal'
var metricFilter  = 'cpu-total'
var rPolicy       = 'default'

This

.pagerDuty()
.serviceKey(pagerdutyKey)

should be formatted like

.pagerDuty()
        .serviceKey(pagerdutyKey)

to show the relation to pagerDuty

nathanielc commented 8 years ago

@mattyjones I like these suggested format changes. In general there is some weirdness with comments currently, just haven't had the time to clean it up.

PRs welcome :)

As for the pagerduty example based on syntax alone you can't determine the indentation, but preserving the original indentation seems like a good middle ground.

That way if you have this:

.pagerDuty()
  .serviceKey(pagerdutyKey)

It is fixed to this:

.pagerDuty()
    .serviceKey(pagerdutyKey)

Or even if you start with this:

.pagerDuty().serviceKey(pagerdutyKey)

Then it remains unmodified. This allows the user to decide how to organize their code, but still provides a consistent format.

mattyjones commented 8 years ago

See when I run tickfmt it does the following:

.pagerDuty()
    .serviceKey(pagerdutyKey)

gets changed to

.pagerDuty()
.serviceKey(pagerdutyKey)

I am running bleeding edge on the whole tick stack

Maybe it does not like my tabs vs spaces? (prefer spaces)

nathanielc commented 8 years ago

@mattyjones Sorry, for the late response. My examples above are how I would like it to work, not how it currently work.