Fix typos and some redundancy#654
Conversation
Fixed a few typos and removed mentions of "from your bash compatible shell" (we use make so these commands are compatible with all clis)
Codecov Report
@@ Coverage Diff @@
## main #654 +/- ##
============================================
- Coverage 84.27% 84.27% -0.01%
+ Complexity 1152 1146 -6
============================================
Files 127 127
Lines 2786 2772 -14
============================================
- Hits 2348 2336 -12
+ Misses 438 436 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Co-authored-by: Brett McBride <[email protected]>
| ```bash | ||
| make all | ||
| ``` | ||
| Note: Copying `.env.dist` to `.env` fixes the warning ```The "PHP_USER" variable is not set. Defaulting to a blank string.``` |
There was a problem hiding this comment.
I think this is really a necessary setup step - can we move it somewhere prominent? I think the very first step after git clone?
There was a problem hiding this comment.
yes, @brettmc, I agree. I wanted to do that but was slightly hesitant. I think it'll be placed better after the git clone
| @@ -142,15 +144,16 @@ In order to update all the vendored libraries in the `/vendor` directory. | |||
|
|
|||
| Once you've made the update to the codebase that you'd like to submit, you may [create a pull request](https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request) to the opentelemetry-php project. | |||
There was a problem hiding this comment.
Could you please also reduce the visual noise of this? eg.:
"If you want to submit changes to the codebase, please create a pull request."
| After you open the pull request, the CI/CD pipeline will run all the associated [github actions](https://github.com/open-telemetry/opentelemetry-php/actions/workflows/php.yml). | ||
|
|
||
| You can simulate the important github actions locally before you submit your PR by running the following command: | ||
| You can simulate the important GitHub actions locally before you submit your PR by running the following command: |
There was a problem hiding this comment.
I still feel this is wrong. This is not simulating GitHub actions, but is simply using the same commands as the CI.
| You can use the [examples/AlwaysOnZipkinToNewrelicExample.php](/examples/AlwaysOnZipkinToNewrelicExample.php) file to test out the reference implementation we have for zipkin to Newrelic. This example performs a sample trace with spans and POSTs the result to a Newrelic endpoint. This requires a license key (free accounts available) to be set in the environment (NEW_RELIC_INSERT_KEY) to authenticate to the backend. | ||
|
|
||
| You can use the [examples/AlwaysOnNewrelicExample.php](/examples/AlwaysOnNewrelicExample.php) file to test out the reference implementation we have for Newrelic. This example perfoms a sample trace with spans and POSTs the result to a Newrelic endpoint. This requires a license key (free accounts available) set in the environment (NEW_RELIC_INSERT_KEY) to authenticate to the backend. | ||
| You can use the [examples/AlwaysOnNewrelicExample.php](/examples/AlwaysOnNewrelicExample.php) file to test out the reference implementation we have for Newrelic. This example performs a sample trace with spans and POSTs the result to a Newrelic endpoint. This requires a license key (free accounts available) set in the environment (NEW_RELIC_INSERT_KEY) to authenticate to the backend. |
There was a problem hiding this comment.
Could you please remove both NewRelic examples. This is not an advertisment platform for NewRelic, and the associated exporters may be moved to the contrib repo anyway.
There was a problem hiding this comment.
Could you please remove both NewRelic examples. This is not an advertisment platform for NewRelic, and the associated exporters may be moved to the contrib repo anyway.
@tidal , Is there a specific reason why we are choosing to remove the NewRelic examples
|
@yuktea |
3 participants
Fixed a few typos and removed mentions of "from your bash compatible shell" (we use make so these commands are compatible with all clis).
A part of [Issue #481]