Skip to content

Add link to requirement levels on Markdown table headers#222

Merged
joaopgrassi merged 12 commits intoopen-telemetry:mainfrom
lmolkova:http-proofreading
Feb 26, 2024
Merged

Add link to requirement levels on Markdown table headers#222
joaopgrassi merged 12 commits intoopen-telemetry:mainfrom
lmolkova:http-proofreading

Conversation

@lmolkova
Copy link
Copy Markdown
Member

@lmolkova lmolkova commented Oct 17, 2023

@lmolkova lmolkova mentioned this pull request Oct 17, 2023
Merged
@lmolkova lmolkova marked this pull request as ready for review October 17, 2023 21:07
@lmolkova lmolkova requested review from a team October 17, 2023 21:07
Oberon00
Oberon00 reviewed Nov 6, 2023
View reviewed changes
@joaopgrassi joaopgrassi changed the title Random editorial improvements Add link to requirement levels on Markdown table headers Nov 21, 2023
@joaopgrassi
Copy link
Copy Markdown
Member

joaopgrassi commented Nov 21, 2023

@lmolkova I took the liberty to update this PR and addressed the issue with the PR title. What is else blocking in merging this?

@lmolkova
Copy link
Copy Markdown
Member Author

lmolkova commented Nov 21, 2023

@lmolkova I took the liberty to update this PR and addressed the issue with the PR title. What is else blocking in merging this?

thanks a lot!!

@lmolkova lmolkova force-pushed the http-proofreading branch 2 times, most recently from 6d6dcf0 to 882376a Compare November 22, 2023 17:06
@lmolkova
Copy link
Copy Markdown
Member Author

lmolkova commented Dec 7, 2023

@open-telemetry/specs-semconv-maintainers can it be merged?

Oberon00
Oberon00 approved these changes Dec 18, 2023
View reviewed changes
@lmolkova
Copy link
Copy Markdown
Member Author

lmolkova commented Feb 18, 2024

@open-telemetry/specs-semconv-maintainers can it be merged?

@joaopgrassi
Copy link
Copy Markdown
Member

joaopgrassi commented Feb 22, 2024 •
edited
Loading

@lmolkova I think we still need to address the comments about the link to the spec https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/attribute-requirement-level.md. That is moved now to semconv.

But if we change the link to the semconv, then other problems appear as I have noted above.

Another idea is to just link to the website instead. Then users avoid to having to click twice to get to the information.

@lmolkova
Copy link
Copy Markdown
Member Author

lmolkova commented Feb 22, 2024

@joaopgrassi great catch and great suggestion!

I changed req level link to use website. We don't have a clear link to attribute types, so I removed link for types.
I think it's ready (yet again)

joaopgrassi
joaopgrassi approved these changes Feb 23, 2024
View reviewed changes
@joaopgrassi joaopgrassi merged commit e966e83 into open-telemetry:main Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Oberon00 Oberon00 Oberon00 approved these changes

+2 more reviewers

@joaopgrassi joaopgrassi joaopgrassi approved these changes

@AlexanderWert AlexanderWert AlexanderWert approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lmolkova @joaopgrassi @Oberon00 @AlexanderWert