-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Move code from markdown to snippets #1540
Conversation
f1763ac
to
db77464
Compare
89acf16
to
c8da56c
Compare
samples/Snippets/Snippets.csproj
Outdated
<ItemGroup> | ||
<PackageReference Include="Polly.Core" /> | ||
<PackageReference Include="Polly.RateLimiting" /> | ||
<PackageReference Include="Polly.Extensions" /> | ||
<PackageReference Include="Polly.Testing" /> | ||
<PackageReference Include="Microsoft.Extensions.Logging.Console" /> | ||
<PackageReference Include="xunit" /> | ||
</ItemGroup> |
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.
Maybe it would be better to reference these from source rather than NuGet? Then if new features are added in the future, you can add docs in the same PR, rather than having to wait until it's been published.
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 was thinking about this, but it would require adding sources to Samples
project and based on ou current flow we are updating the code after the nuget is pushed anyway.
Alternatively, move Snippets under src
folder?
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.
Yeah I think moving the snippets somewhere else is better, as even though we might be doing lots of work now, post v8 it's probably going to be much less frequent in terms of pushes to NuGet.
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.
Are you fine putting them in src/Snippets
?
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.
Sure.
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #1540 +/- ##
=======================================
Coverage 83.92% 83.92%
=======================================
Files 279 279
Lines 6518 6518
Branches 1017 1017
=======================================
Hits 5470 5470
Misses 839 839
Partials 209 209
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
7873d93
to
ec24130
Compare
0ee528d
to
d57f1af
Compare
d57f1af
to
09be533
Compare
09be533
to
2500d3c
Compare
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 work
Details on the issue fix or feature implementation
Contributes to #1091
Confirm the following