search

openzipkin-attic / zipkin-azure

Reporters and collectors for use in Azure
Apache License 2.0
11 stars 9 forks source link

Shortening of the name has made reading the code difficult #13

Closed aliostad closed 7 years ago

aliostad commented 7 years ago

I can see that we have a property name which is actually consumerGroupName. That is confusing for anyone familiar with EventHub. Also processorHost is not the host anymore, it is merely the name of the processorHost (i.e. processorHostName) ... frankly do not agree with these changes.

I agree that for configuration, naming should be short but not sacrificing self-explanatory codes which is important in the upkeep of the .

To have the best of the both worlds, I suggest having shorter duplicate properties that can read and then populate the longer more meaningful names if they are empty.

[2 cents]

codefromthecrypt commented 7 years ago

Also processorHost is not the host anymore, it is merely the name of the processorHost (i.e. processorHostName) ... frankly do not agree with these changes.

so you are saying that a string variable named of processorHost isn't intuitively the name of that host? What other property of processerHost does appending name resolve ambiguity with?

codefromthecrypt commented 7 years ago

Making things consistent with Zipkin is the number one priority of a repo in the openzipkin org. I think you sometimes forget we have a dozen different modules. There's no reason to create unnecessary differences between property and field names.

As you explained at length earlier, when I went for eventhubs (name of the library) and event hub, there's not tons of consistency in microsoft library naming conventions, so I'm not worried about the constructor parameter names.

Let's see if anyone else is bothered by this. I don't want them to change and I think my reasoning is less flimsy.