Documentation & Extensibility#176
Conversation
|
I've removed |
| if (backoffIndex == 5) | ||
| { | ||
| context.Logger.Message("SharpGen internal invalid state on delay: backoffIndex == 5"); | ||
| if (Debugger.IsAttached) Debugger.Break(); | ||
| } | ||
| else if (backoffIndex > 5) | ||
| { | ||
| context.Logger.Message( | ||
| $"SharpGen internal invalid state on delay: backoffIndex = {backoffIndex}"); | ||
| if (Debugger.IsAttached) Debugger.Break(); | ||
| backoffIndex = 0; | ||
| } |
There was a problem hiding this comment.
I kept getting IndexOutOfBoundsException, but couldn't catch it in the debugger. This horrendous block is to assist with debugging, in case it appears again (I stopped hitting it). To be fixed before 2.0 RTW.
There was a problem hiding this comment.
Would using First-Chance Exceptions for IndexOutOfBoundsException enable us to break at the exception throw without having to have this workaround? IIRC that exception isn't enabled for first-chance in VS by default.
There was a problem hiding this comment.
I use Rider for SharpGen work (but e.g. VS for SharpGen SdkTests, I'm not consistent 😄), AFAIK it doesn't support breaking on first chance exceptions. I simplified the hack code though.
| if (backoffIndex == 5) | ||
| { | ||
| context.Logger.Message("SharpGen internal invalid state on reschedule: backoffIndex == 5"); | ||
| if (Debugger.IsAttached) Debugger.Break(); | ||
| } | ||
| else if (backoffIndex > 5) | ||
| { | ||
| context.Logger.Message($"SharpGen internal invalid state on reschedule: backoffIndex = {backoffIndex}"); | ||
| if (Debugger.IsAttached) Debugger.Break(); | ||
| (backoff, backoffIndex, nextDelay) = GenerateBackoff(retryDelay); | ||
| } |
There was a problem hiding this comment.
Same as above.
| private static (TimeSpan[] backoff, int index, TimeSpan? nextDelay) GenerateBackoff(TimeSpan offset) => | ||
| // BUG: 5 should be sufficient, but causes IndexOutOfBounds | ||
| (Backoff.DecorrelatedJitterBackoffV2(offset + TenthOfSecond, 6).ToArray(), 0, null); |
There was a problem hiding this comment.
Note that this backoff formula can produce TimeSpans like (mm:ss, for offset = 00:10): 00:11, 00:26, 03:01, 00:48, ....
| <PackageReference Include="Microsoft.Bcl.HashCode" /> | ||
| <PackageReference Include="System.Text.Json" /> | ||
| <PackageReference Include="System.Runtime.CompilerServices.Unsafe" /> | ||
| <PackageReference Include="Polly" /> |
There was a problem hiding this comment.
Polly is a great project, but feels overengineered and yet still too complex to customize retry/fallback behavior. I dropped the dependency. Polly.Contrib.WaitAndRetry (despite the name) doesn't depend on Polly and provides good exponential backoff formula.
|
Why do we need to include internal Roslyn code in our code base? |
|
Because I didn't want to rewrite complex assembly loading code that was already written and tested in production? |
|
That's reasonable. Can we add a README.md in that folder mentioning that the code is imported from the dotnet/roslyn project? |
|
@jkoritzinsky I can do that, but only in ~8 hours. I'd like to have this reviewed before that, since README is a minor change. |
|
@jkoritzinsky Could you drop CodeFactor requirement (that now blocks even force pushes)? It's very annoying and of little use (external code is brought with no modification). Before I disabled that stupid PowerShell check it was always firing on modifications to Upd: after a bit of trial, error, adding manual exclusions by file in CodeFactor (globs don't work properly for some reason), force-pushing and stuff, I've managed to merge this. The commit referencing this PR below this very comment is one of the failed attempts, disregard. |
Restore the ability to use external documentation providers, but in a new way, similar to Roslyn analyzers and source generators.
andrew-boyarshin
force-pushed
the
return_of_the_jedi
branch
from
6fe1348 to
e91f31c
Compare
andrew-boyarshin
force-pushed
the
return_of_the_jedi
branch
from
e91f31c to
aa694bb
Compare
2 participants
SharpGenTools.Sdk/Extensibilityfolder is mostly based on Roslyn. Plus the wholeSharpGenTools.Sdk/Internal/Roslyn(the only changes are usings and namespace). Also a couple of minor fixes to the changes in my previous PRs (e.g.Hiddenproperty didn't work), needed to make Vortice.Windows build or be documented.Note, that there are additional changes required to make Vortice.Windows be built and documented (in all repos).
P.S. I'm not fixing Roslyn code to pass CodeFactor. Perhaps drop the Git hook that requires it to pass?