elasticsearch-statsd-plugin
elasticsearch-statsd-plugin copied to clipboard
swoop-inc
Automattic's Fork
Howdy,
First of all sorry about this huge pull request, mainly I wanted to get a conversation going to see if you guys would be interested in merging some of the stuff from Automattic's fork of your plugin back in. We've made some extensive changes but mainly this fork:
- Renames some stats to conform with ES native JSON API's naming schemes more
- Is updated to work with multi node clusters without over-reporting index stats
- Reports node stats for all nodes
- Is ES 1.x compatible
Let me know if you guys are interested in merging in any of these features from our fork.
@xyu thanks for doing all this work and preparing a PR: this is awesome and we'd love to evaluate merging the changes. That said, this PR is difficult to review because it has too many commits which have different granularities and are about different logical changes.
Please, consider doing the following:
- Do an interactive rebase to organize commits into logical groups
- Squash minor commits to reduce the total number of commits. It helps to reduce the noise as people look at the git log. Make sure there are good commit messages that describe what's happening and why.
- Submit multiple PRs. Independent PRs can each be based off of master. PRs dependent on other PRs should either wait until the changes are merged or be stacked on top of the previous PRs.
Thanks!
Right of course :) but seeing as there are some rather major changes that break backwards compatibility with both StatsD key names and perhaps with ES 0.90.x (untested) I wanted to see if Swoop would even be interested in this fork.
Are there features you guys would like and other you would rather avoid and how concerned are you guys with backwards compatibility?
@xyu I created the https://github.com/swoop-inc/elasticsearch-statsd-plugin/tree/v1x branch for your PRs to target. Once we are done with the integration there we can switch master to the new version.
