Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add Versioning details #1648
Add Versioning details #1648
Changes from 6 commits
89b9e58
de951fa
0f16444
97a121d
fcd9a06
fc759e1
3b16d87
26b2998
74aa9a0
c8a3432
efa23c0
b3575ee
05cf803
e01bada
1b95c72
0e9061b
5504507
ea29176
dd98a93
8191580
61c4ef2
9620e75
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
One tricky thing is that you may need two policies for
Deprecated
elements;Experimental
will have this attribute removed in the (near) future.Deprecated
and will be removed at some point in the (distant) future.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.
@cijothomas Thanks for taking the time to write this up! I think I'm leaning towards approach 2 for this reason @carlosalberto brings up above. I'm thinking it will be hard for users to know from
[Obsolete]
what is being removed and what is experimental. We could help with the text in the attribute, but I think having the experimental stuff in a pre-release package is more obvious and a better experience for users than combing through warnings. Also, NuGet does support deprecated packages (example Microsoft.CodeAnalysis.FxCopAnalyzers - notice the yellow warning, click for details) which we could possibly use when experimental stuff becomes official and is moved into main libraries?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 think using
[Obsolete]
attribute for experimental features could be confusing, and is probably against the common practice?I would vote for using
[Obsolete]
only on things that we plan to remove in the next major release, not on things that we plan to introduce in the future.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.
There is a variation of Approach1 - ship 1.0.0 release by removing metrics entirely.
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.
Nice! Very good distilled!
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 suspect if there will be a clean cut (e.g.
Sdk.CreateMeterProviderBuilder
). For example, some of the instrumentation libraries might need to have dependency on metrics part. We could end up with a situation where a stable version of instrumentation library needs some metrics functionality which is in the alpha package. And the workaround seems to be painful.