From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id B112B6F3C8; Wed, 5 Oct 2022 23:01:44 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B112B6F3C8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1665000104; bh=Y56g4EIOLXpC6rguVUHSdQcBT0gZf4Y9vVJxefjwE/w=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=PxESU26xV2llPTpUH0bCLVvn540fKjc49fVVN3P21Gm5e6Rgot5xA9IbHMLrckbEw 2dDCmEKQQtlhjiBOMB3s+rZKp4pVK0guMHOxO84BRjbjYFdHrECGm5zvP0Voot5xUS jiupD9H5WavCl9GPP58Qnn/AlXhEdcxYEku6qSrs= Received: from smtpng2.i.mail.ru (smtpng2.i.mail.ru [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 722716F3C8 for ; Wed, 5 Oct 2022 23:01:42 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 722716F3C8 Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1ogAaL-0005Xy-J3; Wed, 05 Oct 2022 23:01:41 +0300 Date: Wed, 5 Oct 2022 22:51:16 +0300 To: Sergey Bronnikov Message-ID: References: <2771fb73-973b-3dd6-ef87-881f31dcda96@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <2771fb73-973b-3dd6-ef87-881f31dcda96@tarantool.org> X-Clacks-Overhead: GNU Terry Pratchett X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD9EA63AAA3C7549E8E188627A532A215A6A01F6E568886BAA200894C459B0CD1B9B5B419EB9538B2445D50FA764FD755AC320824045A51016B5C562C83BEF8E994 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE78CB87876C5D626D4EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006375F0BD5CF353A411D8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D88C9D7713F84E9E395D6A521D951FB062117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC0F49EF363AAD6E82A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735201E561CDFBCA1751F618001F51B5FD3F9D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269176DF2183F8FC7C013A31611C1FE51D268655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7C789C969B8F27C422C4224003CC83647689D4C264860C145E X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34F804B400765FFFF93075DEF7AAB22210248539AF9D24D94403535234C2CF8E9D2ABC049CF48841A01D7E09C32AA3244CA12099FECAD4B0B62E34FF1C620D9AC46C24832127668422927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojSBWEoeHrZs/rRFTNgPOD6Q== X-DA7885C5: 2C537EB25CD9EF00CAA52F6B459461A1C26894B4669EA7E00871425B11E630A4262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F7393CC2E0F076E87284EA59E9C3979F2C4D4E9690464066F1F5DA7C8D0F45F857DBFE9F1EFEE2F478337FB559BB5D741EB964C8C2C849690F8E70A04DAD6CC59E3365FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 1/8] test: introduce LUAJIT_TEST_VARDIR variable X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Igor Munkin via Tarantool-patches Reply-To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Sergey, On 02.09.22, Sergey Bronnikov wrote: > Hi, > > On 31.08.2022 17:53, Igor Munkin wrote: > > Sergey, > > > > Thanks for your review! > > > > On 15.08.22, Sergey Bronnikov wrote: > >> Igor, > >> > >>  thanks for the patch. See my comments inline. > >> > >> On 11.08.2022 14:17, Igor Munkin wrote: > >>> Before the patch both memprof and sysprof artefacts are generated within > >>> the binary artefacts tree (i.e. in the same directory LuaJIT binary is > >>> generated). However, more convenient way is producing these temporary > >>> profiles in a separate directory (e.g. located on the partition with > >>> more strict space limits). As a result of the patch all memprof and > >>> sysprof test chunks consider LUAJIT_TEST_VARDIR environment variable to > >>> set the directory where test profiles are generated. For the case when > >>> LUAJIT_TEST_VARDIR is not set, everything works as before. > >> Commit message is inconsistent a bit with a patch itself. > >> > >> As far as I understand many hunks are not related to introducing > >> LUAJIT_TEST_VARDIR. > >> > >> Probably it is better to change commit one-line message from "test: > >> introduce LUAJIT_TEST_VARDIR variable" > >> > >> to something like "test: refactoring and introduce LUAJIT_TEST_VARDIR > >> variable". It allows to keep patch > >> > >> as is and change expectations for those who will look at your patch. > > I believe your commit subject is too long, so I propose another > > solution. Since the only thing affected by LUAJIT_TEST_VARDIR is > > , so I can write something like "test: introduce > > utils.profilename helper" and describe its rationale and functions > > within the commit message. Are you OK with it? > > I'm ok! I've rewritten the commit message the following way: | test: introduce utils.profilename helper | | Before the patch both memprof and sysprof artefacts were generated | within the binary artefacts tree (i.e. in the same directory LuaJIT | binary is generated). However, more convenient way is producing these | temporary profiles in a separate directory (e.g. located on the | partition with more strict space limits). | | As a result of the patch helper is introduced and | all memprof and sysprof test chunks are patched to use it. The new | helper takes the basename of the profile to be created and also | considers LUAJIT_TEST_VARDIR environment variable to set the directory, | where test profiles are generated. For the case when LUAJIT_TEST_VARDIR | is not set, everything works as before. | | Part of tarantool/tarantool#7472 > > -- Best regards, IM