Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uncover timezone and rename #3083

Merged
merged 4 commits into from Sep 2, 2017
Merged

Conversation

GabrielXia
Copy link
Contributor

Contains

Follows-up to #3075, uncover timezone info in telemetry

@GooeyHub
Copy link
Member

Hooray Jenkins reported success with all tests good!

@GabrielXia GabrielXia mentioned this pull request Aug 27, 2017
@GabrielXia GabrielXia changed the title uncover timezone uncover timezone and rename Aug 28, 2017
@GabrielXia
Copy link
Contributor Author

As mentioned in #3016 Rename MonsterKilled to CreatureKilled :)

@GooeyHub
Copy link
Member

Hooray Jenkins reported success with all tests good!

@GooeyHub
Copy link
Member

Hooray Jenkins reported success with all tests good!

@GabrielXia
Copy link
Contributor Author

In this commit:8db1d35 If a metric such as BlockDestroyedMetric has no data, this is to say, there is no block destroyed record, then we don't send this metric. Does it make sense?

How to test: Don't destroyed blocks in a game, Metrics Menu shows that there is not blocks destroyed. Then exit to the main menu, the BlockDestroyedMetric is not sent to the server. (In before, it will be sent)

@GabrielXia
Copy link
Contributor Author

Update to 04154ee

  1. In fetchMetricAndSend method, found two args not needed, so removed them
  2. Add a filterMetricMap method takes TelemetryConfiguration as a param in stead of bindingMap, so that an other module could use it, e.g. https://github.com/GabrielXia/TelemetryApiTest/blob/master/src/main/java/org/terasology/telemetry/metrics/TelemetryApiTestMetric.java

p.s. I wrote a draft doc about the tutorial module: https://github.com/GabrielXia/TelemetryApiTest/wiki thanks for suggestions :)

@GooeyHub
Copy link
Member

Hooray Jenkins reported success with all tests good!

@Cervator
Copy link
Member

Cervator commented Sep 2, 2017

In this commit:8db1d35 If a metric such as BlockDestroyedMetric has no data, this is to say, there is no block destroyed record, then we don't send this metric. Does it make sense?

How to test: Don't destroyed blocks in a game, Metrics Menu shows that there is not blocks destroyed. Then exit to the main menu, the BlockDestroyedMetric is not sent to the server. (In before, it will be sent)

I'm thinking if 0 is a valid value (which it would be for blocks destroyed) then it should be set as a default value during initialization somewhere. Then it will also qualify for sending, right? Some metrics probably should just have good defaults like that.

Anyway: tested out real quick and merged! Thanks :-)

@Cervator Cervator merged commit 04154ee into MovingBlocks:develop Sep 2, 2017
Cervator added a commit that referenced this pull request Sep 2, 2017
@Cervator Cervator added this to the Alpha 8 milestone Sep 2, 2017
@Cervator Cervator added the Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc. label Sep 2, 2017
@GabrielXia
Copy link
Contributor Author

@Cervator Thanks for the tests and suggestions.

For example, the blocks destroyed metric includes block type -> block destroyed number. The metric will add a new block only if there is a new kind of block destroyed. Do you think we should set some default block type and set their value to 0? If so, which are those default blocks?

@Cervator
Copy link
Member

Cervator commented Sep 2, 2017

Ahh, gotcha, it is a map. Okay. Hmm. At that point I don't know if it matters enough to throw in something special, like 0 destroyed air blocks. Up to you / how it might help the graphs look better :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants