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

Added missing FromConfig.Props(props) #1667

Merged
merged 1 commit into from
Feb 1, 2016
Merged

Added missing FromConfig.Props(props) #1667

merged 1 commit into from
Feb 1, 2016

Conversation

rogeralsing
Copy link
Contributor

Added missing convenience method to FromConfig to comply with the Scala API.

var router1 = ActorOf(FromConfig.Props(Props.Create<Worker>()));
//(same as)
//var router1 = ActorOf(Props.Create<Worker>().WithRouter(FromConfig.Instance));

Scala counterpart:

val router1: ActorRef = context.actorOf(FromConfig.props(Props[Worker]), "router1")

All other router configs do have the corresponding Props method, so it makes sense to be consistent with Scala and the rest of the .NET router configs IMO, even if it is just slight sugar of the existing API.

Also cleaned up the RouterConfig.cs file to comply with C#6 language features.
As all RouterConfig classes uses surrogates for serialization, no such language features affect the serializability of the types.

@skotzko skotzko changed the title Added missing FromConfig.Props(props) Added missing FromConfig.Props(props) Jan 24, 2016
@Aaronontheweb
Copy link
Member

@rogeralsing looks good to me, but you need to follow this process and update the PR to get the API approval specs to pass: http://getakka.net/docs/akka-developers/public-api-changes

@rogeralsing
Copy link
Contributor Author

Something seems bonkers with the API approval stuff, it is merged and still fails

@rogeralsing
Copy link
Contributor Author

According to @Silv3rcircl3 there is an localization issue in the API approval app that mixes up . and , on swe/ger systems

@@ -3769,7 +3769,7 @@ namespace Akka.Routing
}
public class DefaultResizer : Akka.Routing.Resizer, System.IEquatable<Akka.Routing.DefaultResizer>
{
public DefaultResizer(int lower, int upper, int pressureThreshold = 1, double rampupRate = 0.2, double backoffThreshold = 0.3, double backoffRate = 0.1, int messagesPerResize = 10) { }
public DefaultResizer(int lower, int upper, int pressureThreshold = 1, double rampupRate = 0,2, double backoffThreshold = 0,3, double backoffRate = 0,1, int messagesPerResize = 10) { }
Copy link
Contributor

Choose a reason for hiding this comment

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

There it is :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ouch it modifies the code to non valid C#....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Running with UK settings now :-)

@rogeralsing
Copy link
Contributor Author

F# this.. : (╯°□°)╯︵ ┻━┻

I cant get the API approval stuff to work

…cala API.

```csharp
var router1 = ActorOf(FromConfig.Props(Props.Create<Worker>()))
```

Scala counterpart:

```scala
val router1: ActorRef = context.actorOf(FromConfig.props(Props[Worker]), "router1")
```

Also cleaned up the RouterConfig.cs file to comply with C#6 language features.
As all RouterConfig classes uses surrogates for serialization, no such language features affect the serializability of the types.

api

approved

api
@Aaronontheweb
Copy link
Member

Problem with the diff software or which part?

@rogeralsing
Copy link
Contributor Author

Its the API diff software that doesnt handle localization, many non US/UK devs will see a broken approved.txt

I'm running with UK settings to produce proper API approvals ATM.

@Aaronontheweb
Copy link
Member

Bah, sorry- is it an issue with:

  • The diff tool
  • The API file generator

We'll need to hard code a localization setting into one of those. I suspect the latter

@Aaronontheweb
Copy link
Member

On mobile at the moment so my comments might sound a bit off - damn autocorrect

Aaronontheweb added a commit that referenced this pull request Feb 1, 2016
Added missing FromConfig.Props(props)
@Aaronontheweb Aaronontheweb merged commit 0232fc5 into akkadotnet:dev Feb 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants