-
Notifications
You must be signed in to change notification settings - Fork 158
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
Improves readme. #10
Improves readme. #10
Conversation
|
||
### Run tests | ||
- PHP 5.6 or later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Miss bcmath
extension. I know this is still wip but that way I don't forget :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
@@ -18,7 +18,6 @@ | |||
"minimum-stability": "dev", | |||
"require": { | |||
"php": "^5.6||^7.0", | |||
"ext-bcmath": "*", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kevinlebrun removed the dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
README.md
Outdated
'global_tags' => ['host' => 'hostname'], | ||
]; | ||
|
||
$tracer = new Tracer($transport, [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
general design: can't we provide reasonable defaults to avoid all this configuration? it's quite long to configure the tracer. If it already has defaults, let's create an advanced section with this snippet and a simple "Getting Started" where you initialize the tracer with defaults + trace a code-block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tracer comes with all this defaults, the reason why I included it in the example was to show all accepted arguments in the config. We can definitively move them to a advanced config or simply add a link in the bottom to the actual code (which also includes information).
/** | ||
* Enabled, when false, returns a no-op implementation of the Tracer. | ||
*/ | ||
'enabled' => true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this as it was not being used and can not find how is it being used in other libraries. cc @palazzem
c6d4a2a
to
c4a3782
Compare
c4a3782
to
7332068
Compare
No description provided.