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 914BA6EC40; Wed, 18 Aug 2021 11:25:42 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 914BA6EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1629275142; bh=8Zo6k8O3HRPbp5x8YN8zvbVwL1SFLHZxN6z+0jvxRvg=; 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=ZJg/T24xSKSL20xuxYrfYZL4qsVi8j/1eD4SA3YgJwj4sx64Vd0hOb0MFCIoFOvWP MuPulFsO/yJY+TnL1O83ZC2fC6thbZZn16pD9kOlvU9kglsOu4L+eaWtlpIxHNeZBS P7VNYjIp5J1yi+/8NWhqLJfxbQ/ct82PcsmqSugg= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [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 6387F6EC40 for ; Wed, 18 Aug 2021 11:25:40 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 6387F6EC40 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1mGGtH-0001l0-4B; Wed, 18 Aug 2021 11:25:39 +0300 Date: Wed, 18 Aug 2021 11:25:37 +0300 To: Timur Safin Message-ID: <20210818082537.eyhaa2awaabsje56@esperanza> References: <20210818082222.mofgheciutpipelz@esperanza> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210818082222.mofgheciutpipelz@esperanza> X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD9ECFD080E047A606F6525B29142351271182A05F5380850404994484AB3B5809C4ECCEB5AE47760F2C2C5FED783051AFA1886CC030859A83B X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7AED985C8E545F588EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063757004B04402545C58638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8CB402C73312A76FAD4557C1C2BE8F0FC117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BF1175FABE1C0F9B6A471835C12D1D977C4224003CC8364762BB6847A3DEAEFB0F43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B68655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7CB0EC3B1FCAE4A06943847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A513162D1773A64FD5FFDA93D39A94913E8517543CAA05BE97D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA757E10A58996CBD514410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D346C409ABC5F9C579BE7BD4C07F5E0FCF59B86D589D4866BA6D7C8145F8CB5981E653CA1713CA2A0E01D7E09C32AA3244CFF63F3ED717B9094EA08F66BCE935A55795D98D676DD64D08D5DD81C2BAB7D1D X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojuRQ/H5n28tqvNr6WIz3eVg== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D0ADBA8C4629AF6FDC6BEF18BE1C84D24274CEFED1673C562683ABF942079399BFB559BB5D741EB966A65DFF43FF7BE03240331F90058701C67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v5 0/8] Initial datetime implementation 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: Vladimir Davydov via Tarantool-patches Reply-To: cover.1629071531.git.tsafin@tarantool.org Cc: Vladimir Davydov , tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" [ += tarantool-patches - sorry dropped it from Cc by mistake ] Putting aside minor hitches, like bad indentation here and there, I have some concerns regarding the datetime API: - I believe we shouldn't introduce interval in years/months, because they depend on the date they are applied to. Better add datetime methods instead - add_years / add_months. - The API provides too many ways to extract the same information from a datetime object (m, min, minute all mean the same). IMO a good API should provide exactly one way to achieve a certain goal. - AFAICS datetime.hour returns the number of hours passed since the year 1970. This is confusing. I don't have any idea why anyone would need this. As a user I expect them to return the hour of the current day. Same for other similar methods, like month, minute, year. - Creating a datetime without a date (with parse_time) looks weird: tarantool> dt = datetime.parse_time('10:00:00') --- ... tarantool> tostring(dt) --- - 1970-01-01T10:00Z ... Why 1970? I think that a datetime should always be created from a date and optionally a time. If you want just time, you need another kind of object - time. After all it's *date*time. - datetime.days(2) + datetime.hours(12) + datetime.minutes(30) looks cool, but I'm not sure it's efficient to create so many objects and then sum them to construct an interval. It's surely acceptable for compiled languages without garbage collection (like C++), but for Lua I think it'd be more efficient to provide an API like this: datetime.interval{days = 2, hours = 12, minutes = 30} (I'm not a Lua expert so not sure about this) - datetime.strftime, asctime, ctime - look too low level, which is no surprise as they come from the C API. Judging by their names I can't say that they present a datetime as a string in a certain format. Maybe, rename strftime() to format() and make it a method of datetime object (datetime:format)? Without a format, it could print ASCII time. As for asctime, ctime, I'd drop them, because one can use strftime to get the same result. Besides, they append \n to the output, which looks weird: tarantool> datetime.asctime(dt) --- - 'Thu Jan 1 10:00:00 1970 ' ...