Exposes core_id in the configuration of an HttpServer #534
Conversation
|
Also if you could also update the Changelog as the new PR template suggests? In section Breaking, as you are adding a new field to an existing struct which is not marked with non_exhaustive. |
| @@ -141,7 +145,7 @@ impl From<&Configuration> for Newtype<httpd_config_t> { | |||
| ))] | |||
| task_caps: (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT), | |||
| stack_size: conf.stack_size, | |||
| core_id: i32::MAX, | |||
| core_id: conf.core.map(|core| core.into()).unwrap_or(i32::MAX), | |||
There was a problem hiding this comment.
This I'm not sure about because I don't know much about all this, but this should be a core still, no? Shouldn't you do something like this?
conf.core.map(|core| core.into()).unwrap_or(Core::Core0)I'm not sure but it feels like i32::MAX would always crash.
There was a problem hiding this comment.
No, core_id is an i32, and i32::MAX works perfectly fine.
|
This PR looks OK now, but merging it has to wait a bit, because adding a new field to the conf struct is backwards-incompatible (strictly speaking). Fortunately, it is only a matter of a few days. Turns out, we need to release a new non-patch |
No description provided.