Skip to content

sharding c# 7 #3033

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Sep 15, 2017
Merged

sharding c# 7 #3033

merged 6 commits into from
Sep 15, 2017

Conversation

zbynek001
Copy link
Contributor

No description provided.

@Horusiath
Copy link
Contributor

Quick question: Have you changed anything in the implementation itself, you have you just upgraded code style to new C# 7 features?

@zbynek001
Copy link
Contributor Author

nothing else was changed, only adding c# 7 features

Danthar
Danthar reviewed Aug 28, 2017
View reviewed changes
src/contrib/cluster/Akka.Cluster.Sharding.Tests.MultiNode/ClusterShardingSpec.cs
UpdateState(cc);
return true;
}
return false;
Copy link
Member

Choose a reason for hiding this comment

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

@Horusiath Is this a valid replacement for the message.Match().With<> semantics ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes

Copy link
Member

Choose a reason for hiding this comment

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

ok

Copy link
Member

Choose a reason for hiding this comment

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

why the switch statement on only one case? it would be better expressed as an if statement.

@Danthar
Copy link
Member

Danthar commented Aug 28, 2017

Mono build fails with 21 build errors. Relevant log starts around here:
http://petabridge-ci.cloudapp.net/viewLog.html?tab=buildLog&logTab=tree&filter=debug&expand=all&buildId=39173#_focus=1907

@zbynek001
Copy link
Contributor Author

I've checked the errors, it's related to a bug in Roslyn, which was fixed some time ago:
dotnet/roslyn#16935
So maybe there is old sdk on build server or something similar? Not sure

@Aaronontheweb
Copy link
Member

@zbynek001 we just upgraded to the .NET 2.0 SDK today

@zbynek001
Copy link
Contributor Author

@Aaronontheweb I know, that's why I tried updating this. And seems it helped

@Aaronontheweb Aaronontheweb merged commit ccf7b6a into akkadotnet:dev Sep 15, 2017
@zbynek001 zbynek001 deleted the sharding-csharp-7 branch June 24, 2018 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sean-gilliam sean-gilliam sean-gilliam left review comments

@Danthar Danthar Danthar left review comments

+1 more reviewer

@alexvaluyskiy alexvaluyskiy alexvaluyskiy left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@zbynek001 @Horusiath @Danthar @Aaronontheweb @sean-gilliam @alexvaluyskiy