From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id gCgHN/Ms1V7+FgAA0tVLHw (envelope-from ) for ; Mon, 01 Jun 2020 16:29:39 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id mJsgM/Ms1V7pNwAAB5/wlQ (envelope-from ) for ; Mon, 01 Jun 2020 16:29:39 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id AB5C59404CF for ; Mon, 1 Jun 2020 16:29:38 +0000 (UTC) Received: from localhost ([::1]:57320 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jfnJg-0002YN-8Z for larch@yhetil.org; Mon, 01 Jun 2020 12:29:36 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52478) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jfnJD-0002WT-6w for emacs-orgmode@gnu.org; Mon, 01 Jun 2020 12:29:07 -0400 Received: from confino.investici.org ([2a00:c38:11e:ffff::a020]:31511) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jfnJB-0004wO-QP; Mon, 01 Jun 2020 12:29:06 -0400 Received: from mx1.investici.org (unknown [127.0.0.1]) by confino.investici.org (Postfix) with ESMTP id A3AB620F88; Mon, 1 Jun 2020 16:28:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=anche.no; s=stigmate; t=1591028939; bh=XTXZ8gpQaAR2Ou+lKSD7NQMYD0+xQpIpRwmeCkdSYo8=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=q4FPF8IswQsMqP6lxw279c23UbTi0SNkoyGDnHCDwTxNcHdZfTsLKnfYCemkeIbpq OYQOMG+3JluXkTVZr8ZnfUn+J2mlttuR6F1vFKRcqjuGdyRZqX6ZC28LkC6f5LUu7a vrJBvkDfEFWU/HSblsxElZZvoayQae5trVj7rSKk= Received: from [212.103.72.250] (mx1.investici.org [212.103.72.250]) (Authenticated sender: mariotomo@inventati.org) by localhost (Postfix) with ESMTPSA id 53B2B20D87; Mon, 1 Jun 2020 16:28:58 +0000 (UTC) Subject: Re: issue tracker? To: Bastien References: <87ftbw3mvw.fsf@alphaville.usersys.redhat.com> <6ffc8465-fdeb-4981-8303-43f09e26e0c1@www.fastmail.com> <20200519073352.GC7874@tuxteam.de> <08ee796b-d541-69f3-518e-f577d9b29f0b@anche.no> <87imgau9ny.fsf@gnu.org> <71f5fa9a-8f55-6869-151a-209f53f86579@anche.no> <874kruzssr.fsf@gnu.org> From: Mario Frasca Message-ID: Date: Mon, 1 Jun 2020 11:28:48 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.1 MIME-Version: 1.0 In-Reply-To: <874kruzssr.fsf@gnu.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Received-SPF: pass client-ip=2a00:c38:11e:ffff::a020; envelope-from=mario@anche.no; helo=confino.investici.org X-detected-operating-system: by eggs.gnu.org: No matching host in p0f cache. That's all we know. X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001 autolearn=_AUTOLEARN X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: emacs-orgmode@gnu.org Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Scanner: scn0 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=anche.no header.s=stigmate header.b=q4FPF8Is; dmarc=none; spf=pass (aspmx1.migadu.com: domain of emacs-orgmode-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=emacs-orgmode-bounces@gnu.org X-Spam-Score: -0.21 X-TUID: TVL39o/QtCbB On 01/06/2020 10:53, Bastien wrote: > Let us know what would help you contribute more. as mentioned, I would like to correct docstrings, refactor the code in a few points, and add unit tests. --- (defun org-plot/gnuplot-script (data-file num-cols params &optional preface)   "Write a gnuplot script to DATA-FILE respecting the options set in PARAMS. this is not what the function does: DATA-FILE is purely a string, the name of the file containing the data, and the function returns the script as a string, which refers to DATA-FILE. in practice, the author could have left the DATA-FILE parameter altogether, used a placeholder, say %DATAFILE%, and replaced it in the returned string.  but it works, and I would not fix it, except for the docstring, which is misleading. --- (defun org-plot/goto-nearest-table ()   "Move the point to the beginning of nearest table. Moves back if the point is currently inside of table, otherwise forward to next table.  Return value is the point." this is what the function does, but the current docstring says (defun org-plot/goto-nearest-table ()   "Move the point forward to the beginning of nearest table. Return value is the point at the beginning of the table." where "nearest" is not defined. --- collecting options is a candidate for refactoring:       (save-excursion (while (and (equal 0 (forward-line -1))                   (looking-at "[[:space:]]*#\\+"))             (setf params (org-plot/collect-options params)))) this is how it is used, inside of the exposed command org-plot/gnuplot. --- If I understood my other reviewer Kyle Meyer correctly, he was suggesting that usage of setf instead of setq in this source was not exactly appropriate, and I think it is only necessary in one case, the others can be replaced with setq. but then again, fixing something that works and has no unit tests, may be a recipe for future failure. --- there are other minor mistakes in the code and in the documentation, which are quite unrelated, like formatting …       (let ((num-rows (length table)) (num-cols (length (nth 0 table)))         (gnuplot-row (lambda (col row value) notice how this let has three clauses, but they fit on two lines. or simply typing =dep= instead of =deps=. --- I've not been collecting them, this is just the few that I can recall, and would like to correct them, but in order to make my contribution only touch the code I want to add, I refrain from doing so. best regards, MF