Skip to content

add top-level example for binary heap#4331

Merged
burner merged 1 commit intodlang:masterfrom
wilzbach:example_binaryheap
Jun 1, 2016
Merged

add top-level example for binary heap#4331
burner merged 1 commit intodlang:masterfrom
wilzbach:example_binaryheap

Conversation

@wilzbach
Copy link
Contributor

  • adds a top-level example for the binary heap
  • adds support to extend/grow a normal array (didn't see a reason why this shouldn't be allowed)
  • removes the out-commented private (seems like the bug was fixed)

Ping @schveiguy

@wilzbach wilzbach force-pushed the example_binaryheap branch from 8dc8640 to 5ff65fa Compare May 15, 2016 22:54
@schveiguy
Copy link
Member

I need to fix the authorship on these submodules :) I only wrote the RedBlackTree, and Andrei and others wrote everything else!

@burner
Copy link
Member

burner commented May 18, 2016

@wilzbach can you split these kind of PR's either it is a doc PR or it is an code enhancement.
and comments like "seems like the bug was fixed" are just not ok. either it is fixed or not.

IMO this PR should be closed and split into multiple new PR's, each with a singular focus.

@wilzbach wilzbach force-pushed the example_binaryheap branch from 5ff65fa to f8b363b Compare May 25, 2016 02:15
@wilzbach
Copy link
Contributor Author

@wilzbach can you split these kind of PR's either it is a doc PR or it is an code enhancement.
and comments like "seems like the bug was fixed" are just not ok. either it is fixed or not.

Sorry. Ok, makes sense,

IMO this PR should be closed and split into multiple new PR's, each with a singular focus.

I rebased this PR to be just about the added unittest example to the documentation. Hope this reduces the noise a bit ;-)

///
unittest
{
import std.algorithm : equal;
Copy link
Member

Choose a reason for hiding this comment

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

isn't equal in std.algorithm.comparison ? I mean if you use selected imports, use them on deepest nested module.

@burner
Copy link
Member

burner commented May 25, 2016

Other than my remark about the imports LGTM

@wilzbach wilzbach force-pushed the example_binaryheap branch from f8b363b to a3f229f Compare May 25, 2016 14:00
@wilzbach wilzbach force-pushed the example_binaryheap branch 2 times, most recently from 4bf1489 to 5f7a4ca Compare May 25, 2016 14:03
@wilzbach
Copy link
Contributor Author

Other than my remark about the imports LGTM

Rebased, but now it fails, because it depends on the bug fix #4359 :/
-> I removed the insert part

    auto q = heapify!"a > b"([1, 4, 5]);
    q.insert(2);
    assert(q.front == 1);
    q.insert(0);
    assert(q.front == 0);
``

and will the include it with #4359?

@burner
Copy link
Member

burner commented Jun 1, 2016

Auto-merge toggled on

@burner burner merged commit 1e64085 into dlang:master Jun 1, 2016
@wilzbach wilzbach deleted the example_binaryheap branch June 8, 2016 01:33
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.

3 participants