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

Added capability to show obsolete members in diff with message #171

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jamesmcroft
Copy link

A look at implementing a fix for #167, providing diff information for obsolete members.

Change includes extra NumObsolete fields to the existing diff info types and an ObsoleteMessage field to the MemberDiffInfo type.

Updates made to the diff UI to reflect obsolete members showing the message

@mindsolve
Copy link

mindsolve commented Jun 5, 2022

Hi, thank you for that change! This is going to be helpful.

I have just tested the changes and have run into an exception in your code.
My assumption is that the code doesn't handle ObsoleteAttribute calls without arguments.

The package MineStat marked multiple methods as obsolete from 1.x->2.0.0, but did not provide workaround messages (only added in 2.1.1). Comparison between 1.x and 2.1.1 work flawlessly, listing the added deprecations without error.

Stacktrace:

InvalidOperationException: Sequence contains no elements
System.Linq.ThrowHelper.ThrowNoElementsException()
   at System.Linq.ThrowHelper.ThrowNoElementsException()
   at System.Linq.Enumerable.First[TSource](IEnumerable`1 source)
   at FuGetGallery.ApiDiff.ValidateObsolete(ICustomAttributeProvider member) in <$ProjectDir>\FuGetGallery\Data\ApiDiff.cs:line 223
   at FuGetGallery.ApiDiff..ctor(PackageData package, PackageTargetFramework framework, PackageData otherPackage, PackageTargetFramework otherFramework) in <$ProjectDir>\FuGetGallery\Data\ApiDiff.cs:line 174
   at FuGetGallery.ApiDiff.ApiDiffCache.<>c__DisplayClass1_0.<GetValueAsync>b__0() in <$ProjectDir>\FuGetGallery\Data\ApiDiff.cs:line 249
   at System.Threading.Tasks.Task`1.InnerInvoke()
   at System.Threading.Tasks.Task.<>c.<.cctor>b__272_0(Object obj)
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)

To replicate:
call the URL /packages/MineStat/2.0.0/lib/net46/diff/1.0.1/
(Do an API comparison between versions 1.0.1 and 2.0.0 of the package MineStat)

(edit: forgot to include the actual error message 😉)

@jamesmcroft
Copy link
Author

Thanks for the example, I tried with a few that I'd known and all came out okay 😅

I'll take a look 😄

@jamesmcroft
Copy link
Author

@mindsolve that should be solved now. Thanks for testing this 👍🏻

@TylerBrinkley
Copy link
Contributor

Does this show obsolete messages in a non-diff context viewing of the API?

@mindsolve
Copy link

@TylerBrinkley: No, it doesn't. I have implemented that feature in my branch based on this PR, so either @jamesmcroft integrates it in here or I'll open a second PR after this one has been merged :) (#173)

@jamesmcroft
Copy link
Author

@mindsolve @TylerBrinkley happy to look into the non-diff context as part of this change 👍🏻

@jamesmcroft
Copy link
Author

Merged the changes in that @mindsolve added.

This PR should now cover API diff obsolete messaging as well as the non-diff context @TylerBrinkley 👍🏻

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.

3 participants