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 67A8670358; Tue, 16 Feb 2021 18:30:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 67A8670358 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1613489408; bh=tvOQ6yO35zZjJcA+GkdBV8IOlrDOmhxQzzDSu5IJXZ0=; 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=aGOzTzJ11e5xCYzhxXuJ7zVDHAcmhGRgiiHqij4O8CLeE+/9/68duY+GGtl6Wwuoy 6XvPblGK8AEk9G9sdkjFfRpORXrMgvBPLp8X3LYn4EffS/abk83H7oymLtEGtnPrJ9 7ddTxfElShvt7ISS9QL5vrm3wPRX4lxdOD//3oVQ= Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 3882670358 for ; Tue, 16 Feb 2021 18:30:05 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3882670358 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1lC2IZ-0001Xp-9L; Tue, 16 Feb 2021 18:29:59 +0300 Date: Tue, 16 Feb 2021 18:29:56 +0300 To: Sergey Kaplun Message-ID: <20210216152956.GM5448@tarantool.org> References: <2db1d3a53682de0bb4231ec494fc44e95acd9a8f.1612291495.git.imun@tarantool.org> <20210214191626.GF9361@root> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20210214191626.GF9361@root> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.10.1 (2018-07-13) X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD91883A1EE8D2E99327A08A4263FA5D41CBCD47BF76BEDDBF7182A05F5380850401B8D744C0BBDD1796565CDE9BF62638A6D7A9DBBFFF152C20FF893F5C9DFF2B8 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7AD2F2D6F6013FF7FC2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE745C0EDBD94D46193EA1F7E6F0F101C674E70A05D1297E1BBC6CDE5D1141D2B1C33B662ED09A6B4CEBE2EE0D4827AF1B916DD9531C6484A889FA2833FD35BB23D9E625A9149C048EE33AC447995A7AD186FD1C55BDD38FC3FD2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8B5F0C88D684269EDEA471835C12D1D977C4224003CC836476EC64975D915A344093EC92FD9297F6718AA50765F7900637149D0840703ADBE5A7F4EDE966BC389F395957E7521B51C24C7702A67D5C33162DBA43225CD8A89F890A246B268E114EA91E23F1B6B78B78B5C8C57E37DE458B4C7702A67D5C3316FA3894348FB808DB48C21F01D89DB561574AF45C6390F7469DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A5305E71A25D058A32370C7CEC1380F42139E6E84C82E86D03D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75448CF9D3A7B2C848410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34D75CEA0D7A80616EEA2B325259A7FC4522B09B536F8842069BD5D5BFA437CEFAE618019AC6AAD9401D7E09C32AA3244C56BA5AB5FE7CE7741AFDDB3A44AAF61E24AF4FAF06DA24FD927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojiBTwj6noE5cBcmS0e6I45Q== X-Mailru-Sender: 689FA8AB762F73936BC43F508A063822552F418FB8CACE9610460B96113D1C62A7C8D0F45F857DBFE9F1EFEE2F478337FB559BB5D741EB964C8C2C849690F8E70A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 4/5] test: fix warnings found with luacheck in misclib* 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, Thanks for the review! On 14.02.21, Sergey Kaplun wrote: > Hi, Igor! > > Thanks for the patch! > > Looks like I was late for the ball and you got two LGTMs, but here’s my No, you were not! > two cents on the matter, because I've been mentioned. Feel free to > ignore my nitpicks. > > LGTM, except two nitpicks below. Added your tag: | Reviewed-by: Sergey Kaplun > > On 02.02.21, Igor Munkin wrote: > > Since the regular static analysis has not been enabled for the test > > chunks in LuaJIT repository yet, the tests for recently implemented > > features still produce luacheck warnings. > > > > The most of the issues are fixed in scope of the commit > > 8fc103fb1a21c28185a1942e75d8d9485e3aade7 ('test: fix warnings spotted by > > luacheck') and this patch fixes the remaining ones. > > > > Fixes tarantool/tarantool#5631 > > Nit: Looks like 'Closes' or even 'Part of' (considering the next patch), > not 'Fixes'. Why? Sergey pointed out the warnings. The sources and the related warnings are *fixed*. So 'Fixes' looks the most suitable here. > Also, After this patch there are tons of misc-related warnings > by the luacheсk without config. So, I think that this patch should be > squashed with the next one. Yes, these changes are not atomic. I can simply reorder the patches, if you insist, but I definitely don't want to squash them into a single one. The reason is that one is about "enabling" the regular check for LuaJIT-related sources (#5470), but as a result of the applying metrics-related series, the introduced chunks have broken luacheck again. The new warnings are reported in a separate issue (#5631). > Feel free to ignore. > > > Part of tarantool/tarantool#4862 > > Part of tarantool/tarantool#5470 > > Follows up tarantool/tarantool#5187 > > > > Reported-by: Sergey Bronnikov > > Signed-off-by: Igor Munkin > > --- > > .../misclib-getmetrics-capi.test.lua | 22 ++++++--------- > > .../misclib-getmetrics-lapi.test.lua | 28 +++++++++---------- > > 2 files changed, 23 insertions(+), 27 deletions(-) > > > > diff --git a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua > > index 959293d..d54caac 100755 > > --- a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua > > +++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua > > @@ -384,7 +384,7 @@ test:test("strhash", function(subtest) > > assert(new_metrics.strhash_miss - old_metrics.strhash_miss == 0) > > old_metrics = new_metrics > > > > - local str2 = "new".."string" > > + local _ = "new".."string" > > Nit: this `local` is redundant. > Feel free to ignore. There is little sense in storing both strings into a separate local slots, but the code looks kinda "consistent". Ignoring. > > > > > new_metrics = misc.getmetrics() > > assert(new_metrics.strhash_hit - old_metrics.strhash_hit == 19) > > -- > > 2.25.0 > > > > -- > Best regards, > Sergey Kaplun -- Best regards, IM