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

Move code from markdown to snippets #1540

Merged
merged 8 commits into from
Sep 4, 2023
Merged

Move code from markdown to snippets #1540

merged 8 commits into from
Sep 4, 2023

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
.github/workflows/on-push-do-docs.yml Outdated Show resolved Hide resolved
build.cake Show resolved Hide resolved
mdsnippets.json Outdated Show resolved Hide resolved
samples/Snippets/Core/Snippets.cs Outdated Show resolved Hide resolved
samples/Snippets/Core/Snippets.cs Outdated Show resolved Hide resolved
samples/Snippets/Program.cs Outdated Show resolved Hide resolved
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.

samples/Snippets/Testing/Snippets.cs Outdated Show resolved Hide resolved
@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
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

.github/workflows/on-push-do-docs.yml Outdated Show resolved Hide resolved
.github/workflows/on-push-do-docs.yml Outdated Show resolved Hide resolved
@martintmk martintmk enabled auto-merge (squash) September 4, 2023 10:39
@martintmk martintmk merged commit 305ac23 into main Sep 4, 2023
17 checks passed
@martintmk martintmk deleted the mtomka/code-snippets branch September 4, 2023 11:37
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.

2 participants