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

adding PHP documentation for exporters, resources, propagation #2415

Merged
merged 28 commits into from
Mar 1, 2023

Conversation

brettmc
Copy link
Contributor

@brettmc brettmc commented Feb 28, 2023

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

Thanks! Great content overall. Did an editing pass.

content/en/docs/instrumentation/php/exporters.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/exporters.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/exporters.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/exporters.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/exporters.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/exporters.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/propagation.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/resources.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/resources.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/manual.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/manual.md Outdated Show resolved Hide resolved
Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. See inline comments.

content/en/docs/instrumentation/php/manual.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/manual.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/manual.md Outdated Show resolved Hide resolved
---
title: Propagation
description: Context propagation for the PHP API
aliases: [/docs/instrumentation/php/api/propagation]
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop the alias.

Copy link
Contributor

Choose a reason for hiding this comment

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

You dropped too many aliases. This one (in the new file) needed to be removed, not the one in content/en/docs/instrumentation/php/getting-started.md. Restore that alias.

content/en/docs/instrumentation/php/propagation.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/propagation.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/propagation.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/resources.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/resources.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/php/exporters.md Outdated Show resolved Hide resolved
@brettmc
Copy link
Contributor Author

brettmc commented Feb 28, 2023

I think I've addressed all feedback now.

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

One alias that needs to be recovered (was deleted by mistake). Otherwise LGTM, but I'll let @cartermp and @svrnm have the final word about content.

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

Thanks, great docs!

@svrnm
Copy link
Member

svrnm commented Mar 1, 2023

@brettmc can you fix the last open issue from @chalin , then we should be able to merge this.

@chalin
Copy link
Contributor

chalin commented Mar 1, 2023

@brettmc can you fix the last open issue from @chalin , then we should be able to merge this.

The alias was added back, so we're good to go! Merging ...

@chalin chalin merged commit 100e517 into open-telemetry:main Mar 1, 2023
@brettmc brettmc deleted the php-docs branch June 21, 2023 03:14
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.

None yet

4 participants