- Notifications
You must be signed in to change notification settings - Fork481
Add support for ExperimentalAttribute#6971
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Codecov Report
@@ Coverage Diff @@## main #6971 +/- ##======================================= Coverage 96.42% 96.43% ======================================= Files 1410 1410 Lines 335881 335956 +75 Branches 11090 11095 +5 =======================================+ Hits 323879 323971 +92+ Misses 9209 9194 -15+ Partials 2793 2791 -2 |
mavasani left a comment
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.
LGTM, but I’d prefer@jcouv to review it and merge once we have his approval.
jcouv commentedOct 5, 2023
Looking |
| [ID1]C.C() -> void"; | ||
| awaitVerifyNet80CSharpAdditionalFileFixAsync(source,shippedText,unshippedText,fixedUnshippedText); | ||
| } |
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.
Consider also testing removal of Experimental attribute (shipped file contains the experiment, but API no longer marked as experimental) and change of Experimental attribute (however I'm not sure it's a realistic/common scenario)
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.
Currently this will be treated as simply removing an old API and adding a new one. It's not the best, but it should work for now. We can work to improve this experience in the future based on how things go with Roslyn experiments.
jcouv left a comment
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.
LGTM Thanks (iteration 1) with some test suggestions to consider
sharwell commentedOct 5, 2023
@jcouv I'm going to submit new tests based on how this first round goes when I submit a PR with an experiment to Roslyn. |
Uh oh!
There was an error while loading.Please reload this page.
Fixes#6759
Experimental items are shown in API files with a
[diagnosticId]prefix, wherediagnosticIdis the ID provided toExperimentalAttribute.