-
Notifications
You must be signed in to change notification settings - Fork 182
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
Don't render deprecated enum members in semconv tables #1110
Don't render deprecated enum members in semconv tables #1110
Conversation
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 wonder if we'd rather preserve the deprecated ones in the semconv tables but only for limited period of time, i.e. for 2 releases before removing them.
However I don't know if that would be technically doable. Maybe if we were encoding the version that a member/attribute was deprecated, deprecated-1.26.0
.
You're right, we don't have a way to express in which version a property was deprecated. It came up in the past and I created #1117 to track it. If we had an option to remove them after X releases, I'd be happy to keep them around for a bit. Until we have this option, I'd prefer not to render them in the specific semconvs - that's what we do for the attributes. |
@@ -1,10 +1,11 @@ | |||
{% import 'stability.j2' as stability %} | |||
{% macro filter(member) %}{% if snippet_type is not defined or (member.deprecated is none or member.deprecated == "") %}{{ "True" }}{% else %}{{ "False" }}{% endif %}{% endmacro %} |
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.
nit: I might be late to the review, but filter usually filters things out, so I was stumbled upon usage, where we filter
member but still continue to process it later
So semantically it should be False
for good member and True
for deprecated one.
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.
{% macro filter(member) %}{% if snippet_type is not defined or (member.deprecated is none or member.deprecated == "") %}{{ "True" }}{% else %}{{ "False" }}{% endif %}{% endmacro %} | |
{% macro filter_deprecated(member) %}{% if snippet_type is not defined or (member.deprecated is none or member.deprecated == "") %}{{ "False" }}{% else %}{{ "True" }}{% endif %}{% endmacro %} |
something like this
Came up in ##1006 (comment)
Deprecated enum members are not useful in semconv docs and should only appear in the registry.
Merge requirement checklist
[chore]