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 BatArray.min_max #757

Merged
merged 2 commits into from
Jun 8, 2017
Merged

Conversation

UnixJunkie
Copy link
Member

No description provided.

@gasche
Copy link
Member

gasche commented May 31, 2017

BatList.min_max takes a ?cmp:('a -> 'a -> int) parameter to specify an alternative comparison function (but .min and .max don't, weird). Do we want to do the same thing here? I think that the choice you made is also reasonable (and the code seems perfectly mergeable).

@UnixJunkie
Copy link
Member Author

BatList.min_max is a much more recent addition than BatList.(min|max).
If we add a cmp optional parameter (defaulting to Pervasives.compare) that could be added
to all of them: min, max, min_max.

@UnixJunkie
Copy link
Member Author

I think this should be accepted as is (because it is coherent with the rest of the Array module).
I we want to add cmp functions to BatArray.(min|max|min_max), please open an issue (I might do it, but later).

@gasche gasche merged commit 5921e54 into ocaml-batteries-team:master Jun 8, 2017
@UnixJunkie UnixJunkie deleted the array_min_max branch August 14, 2017 01:54
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.

2 participants