From: Alexander Turenko <alexander.turenko@tarantool.org> To: Roman Khabibov <roman.habibov1@yandex.ru> Cc: tarantool-patches@freelists.org, Kirill Shcherbatov <kshcherbatov@tarantool.org> Subject: [tarantool-patches] Re: [PATCH] box: add a method to check if transaction is open Date: Wed, 19 Sep 2018 16:38:31 +0300 [thread overview] Message-ID: <20180919133830.bvtnqh3ftbdukobh@tkn_work_nb> (raw) In-Reply-To: <fabb5166-98f4-14aa-c33a-a3b4027af54e@tarantool.org> Hi, Roman! Sorry for the late response. Please, consider comments below. WBR, Alexander Turenko. On Tue, Sep 11, 2018 at 02:51:19PM +0300, Kirill Shcherbatov wrote: > Now LGTM. Sasha, please, take a look. > > >> commit 14fa90d195dc84df21a41c3172d2f0a2e0ca68e2 > >> Author: Roman Khabibov <roman.habibov1@yandex.ru> > >> Date: Fri Sep 7 02:54:28 2018 +0300 > >> > >> box: add a method to check if txn is open > >> > >> Closes: #3518. > >> > >> +box.is_in_txn = function() > >> + return { builtin.box_txn() } > >> +end > >> + Why do you return a table with boolean, but not boolean? > >> diff --git a/test/engine/savepoint.result b/test/engine/savepoint.result > >> index a62a2e1..00d281b 100644 > >> --- a/test/engine/savepoint.result > >> +++ b/test/engine/savepoint.result > >> @@ -14,7 +14,7 @@ s1 = box.savepoint() > >> ... > >> box.rollback_to_savepoint(s1) > >> --- > >> -- error: 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)' > >> +- error: 'builtin/box/schema.lua:346: Usage: box.rollback_to_savepoint(savepoint)' > >> ... > >> box.begin() s1 = box.savepoint() > >> --- > >> @@ -323,27 +323,27 @@ test_run:cmd("setopt delimiter ''"); > >> ok1, errmsg1 > >> --- > >> - false > >> -- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)' > >> +- 'builtin/box/schema.lua:346: Usage: box.rollback_to_savepoint(savepoint)' > >> ... > >> ok2, errmsg2 > >> --- > >> - false > >> -- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)' > >> +- 'builtin/box/schema.lua:346: Usage: box.rollback_to_savepoint(savepoint)' > >> ... > >> ok3, errmsg3 > >> --- > >> - false > >> -- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)' > >> +- 'builtin/box/schema.lua:346: Usage: box.rollback_to_savepoint(savepoint)' > >> ... > >> ok4, errmsg4 > >> --- > >> - false > >> -- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)' > >> +- 'builtin/box/schema.lua:346: Usage: box.rollback_to_savepoint(savepoint)' > >> ... > >> ok5, errmsg5 > >> --- > >> - false > >> -- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)' > >> +- 'builtin/box/schema.lua:346: Usage: box.rollback_to_savepoint(savepoint)' > >> ... > >> s:select{} > >> --- > > It is better to use output filtering feature of test-run to avoid such redundant diffs. Consider test/app/socket.test.lua: ``` 12 env = require('test_run') 13 test_run = env.new() 14 test_run:cmd("push filter '(error: .builtin/.*[.]lua):[0-9]+' to '\\1'") ``` It is not a blocker for that patch, but if you intend to include it, do it in the separate commit before 'add a method to check if txn is open'.
next prev parent reply other threads:[~2018-09-19 13:38 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-09-07 1:00 [tarantool-patches] " Roman Khabibov 2018-09-10 6:21 ` [tarantool-patches] " Kirill Shcherbatov 2018-09-11 0:23 ` roman.habibov1 2018-09-11 0:25 ` roman.habibov1 2018-09-11 11:51 ` Kirill Shcherbatov 2018-09-19 13:38 ` Alexander Turenko [this message] 2018-09-21 0:54 ` [tarantool-patches] Re: [PATCH 1/1] " roman.habibov1 2018-09-21 2:41 ` Alexander Turenko 2018-09-21 22:29 ` roman.habibov1 2018-09-22 1:00 ` Alexander Turenko 2018-09-23 10:11 ` Vladislav Shpilevoy 2018-09-21 0:54 ` [tarantool-patches] Re: [PATCH 2/2] test: add output filtering feature roman.habibov1 2018-09-21 2:53 ` Alexander Turenko 2018-09-21 22:25 ` roman.habibov1 2018-09-24 8:38 ` [tarantool-patches] Re: [PATCH] box: add a method to check if transaction is open Kirill Yukhin
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=20180919133830.bvtnqh3ftbdukobh@tkn_work_nb \ --to=alexander.turenko@tarantool.org \ --cc=kshcherbatov@tarantool.org \ --cc=roman.habibov1@yandex.ru \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH] box: add a method to check if transaction is open' \ /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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox