Clean up the Polly code base

[SeanFarrow]

It would be nice to clean up the polly code base and in the process use C# 6+ features.

I'm intending to do the following:
Update all the Polly projects to use C# Language version 7.3.

Use features of C# 6+ including expression-bodied functions etc.
The only area of the codebase I will not be changing are the execute methods in the Polly policies due to issue #271.

[moerwald]

I fired a small pull request #562. Is that what this topic should handle? Am I on the rigth way?

Thx

[SeanFarrow]

@ moerwald Not at this stage, no. We are not using local functions until the issue (#271) mentioned in #557 is addressed, feel free to use expression-bodied functions/other C#6/7 features though.

[moerwald]

Thx for clarification

[SeanFarrow]

I don’t know whether this means you are still up for helping, but if so, when you submit a change to this PR, could you please indicate the affected areas/policies/what has changed as this makes it easier to manage. Cheers Sean.

[reisenberger]

I'd like to suggest please that we pause any changes between Action, local function and local method, until we have some concrete benchmarks supporting the change. Some early benchmarks I ran did not immediately support the change.

Further, in the doNothing case, other optimisations are also possible and should be explored: eg Action doNothing = null; with onRetry?.Invoke(...) (or similar) syntax elsewhere.

[moerwald]

@SeanFarrow yes I'm still up for helping. Please decline my pull request #562. I'll create a new one.

[moerwald]

@SeanFarrow new pull request for Registry Policy -> #564.

[reisenberger]

Thanks @SeanFarrow and @moerwald !

Please be sure to branch from the head of the latest v7.0.0 dev branch for any code tidy-ups! There are some significant refactors to the architecture of all Polly policies in the v7.0.0 branch; any tidy-ups started from the current master code may well not merge.

Thanks again.

[SeanFarrow]

Should we use the existing Pr?

[reisenberger]

Thanks again @SeanFarrow and @moerwald for everything you are doing on clean-ups!

Any C#6/7 tidy-ups are great (except for changing between lambdas/local functions etc as discussed - will try to post some benchmarks on that), and so long as things are (currently) based on the v7.0.0 branch.

CachePolicy is the other one probably not to touch at the moment, as we have an open PR on that; the PR itself is good to merge, but I want to be sure we have the corresponding changes to Polly cache providers and serializers described in this comment ready before we merge cache changes in core Polly.

Thanks again!

[SeanFarrow]

@ Reisenberger, That’s nice and a PR regarding caching I didn’t even know about, but is really useful. Have you got any objections to updating the required C# language version to v7.3? If not, I’ll do that tomorrow/this weekend and submit a Pr. Some changes might not make the initial v7 branch as I’m doing work with other projects as well, so it’s a question of low-hanging fruit. Definitely happy to sort the cache providers/serializers out once all the other PR’s are merged.

[reisenberger]

Have you got any objections to updating the required C# language version to v7.3?

hi Sean. Can't see any reason not to do this, and think our build.cake and AppVeyor configuration will already handle it. If you want to prove it out before investing time actually introducing C# 7.3 features, maybe make a separate PR just switching to C# 7.3 first, and we can see if the appveyor config just handles it or needs any work?

Thanks again!

[SeanFarrow]

I’ll sort a separate PR out tonight/tomorrow which will prove things out.

[reisenberger]

Closing due to no recent activity.