Skip to content

Feature/metricks#7

Closed
rslota wants to merge 6 commits intomasterfrom
feature/metricks
Closed

Feature/metricks#7
rslota wants to merge 6 commits intomasterfrom
feature/metricks

Conversation

@rslota
Copy link
Copy Markdown
Contributor

@rslota rslota commented Mar 6, 2017

Add metrics support to MongoosePush. For now this PR is only for build on travis.

{:exometer, github: "PSPDFKit-labs/exometer"},
{:elixometer, github: "pinterest/elixometer"},


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be no more than 1 consecutive blank lines.

defmodule MongoosePush.Metrics do
use Elixometer

def update(return_value, metric_prefix, value \\ 1) do
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functions should have a @SPEC type specification.

@@ -0,0 +1,24 @@
defmodule MongoosePush.Metrics do
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modules should have a @moduledoc tag.

push_result = module.push(notification, device_id, worker, opts)

push_result
|> MongoosePush.Metrics.update(~s"push.#{service}.#{mode}")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nested modules could be aliased at the top of the invoking module.

@mongoosepushbot
Copy link
Copy Markdown
Contributor

Ebert has finished reviewing this Pull Request and has found:

  • 4 possible new issues (including those that may have been commented here).

You can see more details about this review at https://ebertapp.io/github/esl/MongoosePush/pulls/7.

@rslota rslota closed this Mar 8, 2017
@rslota rslota deleted the feature/metricks branch March 9, 2017 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants