Tarantool development patches archive
 help / color / mirror / Atom feed
From: Oleg Babin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>,
	tarantool-patches@dev.tarantool.org,
	yaroslav.dynnikov@tarantool.org
Subject: Re: [Tarantool-patches] [PATCH 9/9] util: introduce binary heap data structure
Date: Thu, 11 Feb 2021 09:51:09 +0300
Message-ID: <d3e74a4e-a50a-f605-a66a-4406c29f127a@tarantool.org> (raw)
In-Reply-To: <05a72aa4-39d9-1009-ba4e-48ce7b105d35@tarantool.org>

Thanks for your fixes!

I found you've missed to add new file to "vshard/CMakeLists.txt" [1]


[1] https://github.com/tarantool/vshard/blob/master/vshard/CMakeLists.txt#L9

On 11/02/2021 01:36, Vladislav Shpilevoy wrote:
> Thanks for the review!
>
> On 10.02.2021 10:01, Oleg Babin wrote:
>> Thanks for your patch.
>>
>> Shouldn't it be added to storage "MODULE_INTERNALS" ?
> Hm. Not sure I understand. Did you mean 'vshard_modules' variable in
> storage/init.lua? Why? The heap is not used in storage/init.lua and
> won't be used there directly in future patches. The next patches
> will introduce new modules for storage/, which will use the heap,
> and will reload it.
>
> Also it does not have any global objects. So it does not
> need its own global M, if this is what you meant.

Yes, thanks for your answer. Got it.

>>> diff --git a/test/unit-tap/heap.test.lua b/test/unit-tap/heap.test.lua
>>> new file mode 100755
>>> index 0000000..8c3819f
>>> --- /dev/null
>>> +++ b/test/unit-tap/heap.test.lua
>>> @@ -0,0 +1,310 @@
>>> +#!/usr/bin/env tarantool
>>> +
>>> +local tap = require('tap')
>>> +local test = tap.test("cfg")
>>> +local heap = require('vshard.heap')
>>> +
>> Maybe it's better to use single brackets everywhere: test("cfg") -> test('cfg'). Or does such difference have some sense?
> Yeah, didn't notice it. Here is the diff:
>
> ====================
> diff --git a/test/unit-tap/heap.test.lua b/test/unit-tap/heap.test.lua
> index 8c3819f..9202f62 100755
> --- a/test/unit-tap/heap.test.lua
> +++ b/test/unit-tap/heap.test.lua
> @@ -1,7 +1,7 @@
>   #!/usr/bin/env tarantool
>   
>   local tap = require('tap')
> -local test = tap.test("cfg")
> +local test = tap.test('cfg')
>   local heap = require('vshard.heap')
>   
>   --
> @@ -109,7 +109,7 @@ local function test_min_heap_basic(test)
>           until not next_permutation(indexes)
>       end
>   
> -    test:ok(true, "no asserts")
> +    test:ok(true, 'no asserts')
>   end
>   
>   --
> @@ -143,7 +143,7 @@ local function test_max_heap_basic(test)
>           until not next_permutation(indexes)
>       end
>   
> -    test:ok(true, "no asserts")
> +    test:ok(true, 'no asserts')
>   end
>   
>   --
> @@ -178,7 +178,7 @@ local function test_min_heap_update_top(test)
>           until not next_permutation(indexes)
>       end
>   
> -    test:ok(true, "no asserts")
> +    test:ok(true, 'no asserts')
>   end
>   
>   --
> @@ -219,7 +219,7 @@ local function test_min_heap_update(test)
>           end
>       end
>   
> -    test:ok(true, "no asserts")
> +    test:ok(true, 'no asserts')
>   end
>   
>   --
> @@ -257,7 +257,7 @@ local function test_max_heap_delete(test)
>           end
>       end
>   
> -    test:ok(true, "no asserts")
> +    test:ok(true, 'no asserts')
>   end
>   
>   local function test_min_heap_remove_top(test)
> @@ -273,7 +273,7 @@ local function test_min_heap_remove_top(test)
>       end
>       assert(h:count() == 0)
>   
> -    test:ok(true, "no asserts")
> +    test:ok(true, 'no asserts')
>   end
>   
>   local function test_max_heap_remove_try(test)
> @@ -294,7 +294,7 @@ local function test_max_heap_remove_try(test)
>       assert(obj.index == -1)
>       assert(h:count() == 1)
>   
> -    test:ok(true, "no asserts")
> +    test:ok(true, 'no asserts')
>   end
>   
>   test:plan(7)

  reply	other threads:[~2021-02-11  6:52 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-09 23:46 [Tarantool-patches] [PATCH 0/9] VShard Map-Reduce, part 1, preparations Vladislav Shpilevoy via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 1/9] rlist: move rlist to a new module Vladislav Shpilevoy via Tarantool-patches
2021-02-10  8:57   ` Oleg Babin via Tarantool-patches
2021-02-11  6:50     ` Oleg Babin via Tarantool-patches
2021-02-12  0:09       ` Vladislav Shpilevoy via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 2/9] Use fiber.clock() instead of .time() everywhere Vladislav Shpilevoy via Tarantool-patches
2021-02-10  8:57   ` Oleg Babin via Tarantool-patches
2021-02-10 22:33     ` Vladislav Shpilevoy via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 3/9] test: introduce a helper to wait for bucket GC Vladislav Shpilevoy via Tarantool-patches
2021-02-10  8:57   ` Oleg Babin via Tarantool-patches
2021-02-10 22:33     ` Vladislav Shpilevoy via Tarantool-patches
2021-02-11  6:50       ` Oleg Babin via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 4/9] storage: bucket_recv() should check rs lock Vladislav Shpilevoy via Tarantool-patches
2021-02-10  8:59   ` Oleg Babin via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 5/9] util: introduce yielding table functions Vladislav Shpilevoy via Tarantool-patches
2021-02-10  8:59   ` Oleg Babin via Tarantool-patches
2021-02-10 22:34     ` Vladislav Shpilevoy via Tarantool-patches
2021-02-11  6:50       ` Oleg Babin via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 6/9] cfg: introduce 'deprecated option' feature Vladislav Shpilevoy via Tarantool-patches
2021-02-10  8:59   ` Oleg Babin via Tarantool-patches
2021-02-10 22:34     ` Vladislav Shpilevoy via Tarantool-patches
2021-02-11  6:50       ` Oleg Babin via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 7/9] gc: introduce reactive garbage collector Vladislav Shpilevoy via Tarantool-patches
2021-02-10  9:00   ` Oleg Babin via Tarantool-patches
2021-02-10 22:35     ` Vladislav Shpilevoy via Tarantool-patches
2021-02-11  6:50       ` Oleg Babin via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 8/9] recovery: introduce reactive recovery Vladislav Shpilevoy via Tarantool-patches
2021-02-10  9:00   ` Oleg Babin via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 9/9] util: introduce binary heap data structure Vladislav Shpilevoy via Tarantool-patches
2021-02-10  9:01   ` Oleg Babin via Tarantool-patches
2021-02-10 22:36     ` Vladislav Shpilevoy via Tarantool-patches
2021-02-11  6:51       ` Oleg Babin via Tarantool-patches [this message]
2021-02-12  0:09         ` Vladislav Shpilevoy via Tarantool-patches
2021-03-05 22:03   ` Vladislav Shpilevoy via Tarantool-patches
2021-02-09 23:51 ` [Tarantool-patches] [PATCH 0/9] VShard Map-Reduce, part 1, preparations Vladislav Shpilevoy via Tarantool-patches
2021-02-12 11:02   ` Oleg Babin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d3e74a4e-a50a-f605-a66a-4406c29f127a@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=olegrok@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --cc=yaroslav.dynnikov@tarantool.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git