Skip to content

recommended_domain_count: unit -> int#11545

Merged
xavierleroy merged 1 commit intoocaml:trunkfrom
gasche:dynamic-domain-count
Sep 21, 2022
Merged

recommended_domain_count: unit -> int#11545
xavierleroy merged 1 commit intoocaml:trunkfrom
gasche:dynamic-domain-count

Conversation

@gasche
Copy link
Copy Markdown
Member

@gasche gasche commented Sep 14, 2022

Presenting this as a function rather than a constant in the stdlib
leaves us with more implementation options open in the future.
@gasche gasche force-pushed the dynamic-domain-count branch from e1b669e to bcc0f32 Compare September 14, 2022 08:47
Copy link
Copy Markdown
Member

@damiendoligez damiendoligez left a comment

Choose a reason for hiding this comment

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

LGTM

@xavierleroy
Copy link
Copy Markdown
Contributor

@Octachron: is it still time to have this change in 5.0 ? Otherwise we'll have an incompatibility between 5.0 and 5.1.

@Octachron
Copy link
Copy Markdown
Member

We really want this change in 5.0: it is better to break the API before the first 5.0 beta and hope that no one has yet a critical system in production that depends on the 5.0~alpha1. That might be less true if we wait for 5.1 .

Copy link
Copy Markdown
Contributor

@xavierleroy xavierleroy left a comment

Choose a reason for hiding this comment

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

OK, let's have this in 5.0.

@xavierleroy xavierleroy merged commit a73df93 into ocaml:trunk Sep 21, 2022
xavierleroy pushed a commit that referenced this pull request Sep 21, 2022
Presenting this as a function rather than a constant in the stdlib
leaves us with more implementation options open in the future.

(cherry picked from commit a73df93)
@xavierleroy
Copy link
Copy Markdown
Contributor

Cherry-picked to 5.0: 4a60998

@xavierleroy xavierleroy added this to the 5.0 milestone Sep 21, 2022
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.

5 participants