Skip to content

Conversation

@martintmk
Copy link
Contributor

@martintmk martintmk commented Sep 1, 2023

Details on the issue fix or feature implementation

Contributes to #1091

Confirm the following

  • I started this PR by branching from the head of the default branch
  • I have targeted the PR to merge into the default branch
  • I have included unit tests for the issue/feature
  • I have successfully run a local build

@martintmk martintmk force-pushed the mtomka/code-snippets branch 4 times, most recently from f1763ac to db77464 Compare September 4, 2023 07:03
@martintmk martintmk marked this pull request as ready for review September 4, 2023 07:04
@martintmk martintmk added the v8 Issues related to the new version 8 of the Polly library. label Sep 4, 2023
@martintmk martintmk added this to the v8.0.0 milestone Sep 4, 2023
Comment on lines 10 to 17
<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>
Copy link
Member

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.

Copy link
Contributor Author

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?

Copy link
Member

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.

Copy link
Contributor Author

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 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

@codecov
Copy link

codecov bot commented Sep 4, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (937bf26) 83.92% compared to head (2f8a4b7) 83.92%.
Report is 3 commits behind head on main.

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           
Flag Coverage Δ
linux 83.92% <ø> (ø)
macos 83.92% <ø> (ø)
windows 83.92% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@martincostello martincostello mentioned this pull request Sep 4, 2023
@martintmk martintmk mentioned this pull request Sep 4, 2023
4 tasks
@martintmk martintmk force-pushed the mtomka/code-snippets branch 8 times, most recently from 7873d93 to ec24130 Compare September 4, 2023 09:24
@martintmk martintmk force-pushed the mtomka/code-snippets branch 3 times, most recently from 0ee528d to d57f1af Compare September 4, 2023 09:47
@martintmk martintmk force-pushed the mtomka/code-snippets branch from d57f1af to 09be533 Compare September 4, 2023 10:01
@martintmk martintmk force-pushed the mtomka/code-snippets branch from 09be533 to 2500d3c Compare September 4, 2023 10:02
Copy link
Member

@martincostello martincostello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v8 Issues related to the new version 8 of the Polly library.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants