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

refs #176 #332

Merged
merged 2 commits into from
Feb 6, 2018
Merged

refs #176 #332

merged 2 commits into from
Feb 6, 2018

Conversation

noma4i
Copy link
Contributor

@noma4i noma4i commented Jan 30, 2018

Add valuable information which is hidden in the pile of issues

Add valuable information which is hidden in the pile of issues
@palazzem
Copy link
Contributor

Hello @noma4i and thanks for this suggestion! Actually we describe the Tracer configuration in this section: http://gems.datadoghq.com/trace/docs/#Configure_the_tracer

Do you think it's better to state something in the Quickstart (integration) section? Maybe we can be a bit more explicit.

@palazzem palazzem added docs Involves documentation community Was opened by a community member labels Jan 30, 2018
@noma4i
Copy link
Contributor Author

noma4i commented Jan 30, 2018

@palazzem I know that yard docs contain that. On the other hand readme should contain base configuration suitable to jumpstart with. On/Off switch is valuable info. I think that it needs to be in readme as I came here accross issues and amount of people wondering how to disable tracer with new config is bigger then you think ;)

@palazzem
Copy link
Contributor

Sure thing @noma4i ! consider that I'm collecting feedbacks to improve better our documentation. Let's keep it in the Quickstart section then.

README.md Outdated
@@ -76,6 +76,7 @@ you can activate it. The example above would become:
require 'active_record'

Datadog.configure do |c|
c.tracer enabled: false # optional flag to disable tracer
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a quickstart and the default "expected behavior" is to have the tracer enabled (or at least we hope!), can we move this to a different section?

Before "To know if a given framework or lib is supported by our client, please consult our integrations list." we may add:

To configure the Datadog Tracer, you can define the `configure` block as follows:
Datadog.configure do |c|
  c.tracer enabled: false, hostname: 'trace-agent.local'
  # [...]
end
For a list of available options, check the [Tracer documentation](http://gems.datadoghq.com/trace/docs/#Configure_the_tracer).

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! as soon you update the PR I'm going to merge it! thank you very much for pointing out this important issue about documentation!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Iv pushed changes. Looks way better

@delner delner added this to the 0.11.3 milestone Feb 5, 2018
@palazzem
Copy link
Contributor

palazzem commented Feb 6, 2018

@noma4i thank you very much! going to merge that!

@palazzem palazzem merged commit 39c7303 into DataDog:master Feb 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Was opened by a community member docs Involves documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants