search

chad / turbulence

Hopefully-meaningful metrics
http://chadfowler.com
MIT License
240 stars 28 forks source link

Remove use of class names as metrics_hash keys #34

Closed kerrizor closed 10 years ago

kerrizor commented 10 years ago

As a follow on to the previous PR fixing the bug caused by using class names as hash keys, I took a stab at removing the use of class names as the metrics_hash's keys, replacing them with a type attribute on the generator. I had originally decided to spend my weekend addressing #20 before fixing bugs, and decided that it might be best to have both the original churn calculator as well as a new one based on the Churn gem, as the Churn gem doesn't support perforce (and I don't know enough about it to work on a PR - sorry, @danmayer!)

One interesting wrinkle to this change is that it would allow for the addition of Calculators of many different metric types. Mashing up metrics might be a useful 2.0 feature..

I'm not 100% on this change, and am actively seeking feedback.

danmayer commented 10 years ago

I do think this would likely make it a bit easier to try to build a way to swap out for churn calculator but there is likely some work to be done there... I did recently get a PR with some nice refactorings to clean up the churn gem a bit more as well.

Does anyone still use the perforce support?

kerrizor commented 10 years ago

I don't know anyone who uses it - @jhwist thoughts? If I were to drop it, I'd want to do so in a 2.x release.

danmayer commented 10 years ago

put out a tweet to see if we could get any feedback from rubyist or turbulence users.

https://twitter.com/danmayer/status/487435430933970944

jhwist commented 10 years ago

I use Perforce, but it's not mission critical to have support in turbulence; I'm fine if support was dropped in a 2.0 release, if that makes adding the churn gem easier. In fact, I had long planned to do a pull request for perforce support for churn, so that way it'd come back. So: go for it.

kerrizor commented 10 years ago

Thanks for the feedback, @jhwist!

danmayer commented 10 years ago

@jhwist awesome thanks and I would love help adding Perforce to churn, I just never use perforce so I found it was fairly difficult for me to implement myself.