Skip to content

Add explanation to the doc of Hashtbl.create#13535

Merged
Octachron merged 1 commit into
ocaml:trunkfrom
MisterDA:hashtbl-create-doc
Oct 21, 2024
Merged

Add explanation to the doc of Hashtbl.create#13535
Octachron merged 1 commit into
ocaml:trunkfrom
MisterDA:hashtbl-create-doc

Conversation

@MisterDA

@MisterDA MisterDA commented Oct 9, 2024

Copy link
Copy Markdown
Contributor

Majority view in the triage discussion of #13474 concluded that we should allow negative values but document that they are disregarded.
Fix #13469.
cc @NickBarnes

@ghost

ghost commented Oct 9, 2024

Copy link
Copy Markdown

What about stdlib/moreLabels.mli and stdlib/templates/hashtbl.template.mli?

(* NOTE: If this file is hashtbl.mli, do not edit it directly! Instead,
   edit templates/hashtbl.template.mli and run tools/sync_stdlib_docs *)

@MisterDA MisterDA force-pushed the hashtbl-create-doc branch from c7a3912 to bc1e7f0 Compare October 9, 2024 09:33
@MisterDA

MisterDA commented Oct 9, 2024

Copy link
Copy Markdown
Contributor Author

What about stdlib/moreLabels.mli and stdlib/templates/hashtbl.template.mli?

Ooops, I cherry-picked Nick's draft and didn't check this. Fixed, thanks!

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM now

Comment thread stdlib/hashtbl.mli Outdated
the table. The table grows as needed, so [n] is just an
initial guess.
(** [Hashtbl.create n] creates a new, empty hash table, with suggested
initial size [n]. For best results, [n] should be on the order of

@Octachron Octachron Oct 10, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not sure if suggested is the right term here: the table is created with a concrete size. Maybe

[Hashtbl.create n] creates a new, empty hash table, with initial size greater or equal to the suggested size [n].

?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I've applied your suggestion.

@Octachron Octachron left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The updated documentation looks good to me. @MisterDA , could you add a Changes entry in the documentation section of 5.3 ?

@Octachron Octachron added this to the 5.3 milestone Oct 18, 2024
Majority view in the triage discussion of ocaml#13474 concluded that we
should allow negative values but document that they are disregarded.
@MisterDA

Copy link
Copy Markdown
Contributor Author

sorry, the documentation text was off by one space.

@Octachron Octachron merged commit 6c946cd into ocaml:trunk Oct 21, 2024
@MisterDA MisterDA deleted the hashtbl-create-doc branch October 21, 2024 12:56
Octachron added a commit that referenced this pull request Oct 23, 2024
Add explanation to the doc of `Hashtbl.create`

(cherry picked from commit 6c946cd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hashtbl.create with negative initial guess

3 participants