From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Hanno Perrey Newsgroups: gmane.emacs.devel Subject: Re: [ELPA] New package: jami-bot and org-jami-bot Date: Sun, 14 Jan 2024 19:46:07 +0100 Message-ID: <87cyu3aftq.fsf@hoowl.se> References: <875y0i7e43.fsf@hoowl.se> <87y1cyqoso.fsf@posteo.net> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="32879"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Stefan Kangas , emacs-devel@gnu.org To: Philip Kaludercic Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Jan 14 20:31:18 2024 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1rP6CU-0008SN-1S for ged-emacs-devel@m.gmane-mx.org; Sun, 14 Jan 2024 20:31:18 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rP6Bf-0005Dy-Pc; Sun, 14 Jan 2024 14:30:27 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rP5vd-0002Ln-Of for emacs-devel@gnu.org; Sun, 14 Jan 2024 14:13:53 -0500 Original-Received: from mx1.riseup.net ([198.252.153.129]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rP5vW-0000wL-L6 for emacs-devel@gnu.org; Sun, 14 Jan 2024 14:13:50 -0500 Original-Received: from fews02-sea.riseup.net (fews02-sea-pn.riseup.net [10.0.1.112]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx1.riseup.net (Postfix) with ESMTPS id 4TClLG3DYtzDqM3; Sun, 14 Jan 2024 19:13:42 +0000 (UTC) X-Riseup-User-ID: F938E6B51DFA171394129448497D195DB5071DC04FD0670653494F34F19FC767 Original-Received: from [127.0.0.1] (localhost [127.0.0.1]) by fews02-sea.riseup.net (Postfix) with ESMTPSA id 4TClLF5QSjzFttY; Sun, 14 Jan 2024 19:13:41 +0000 (UTC) In-reply-to: <87y1cyqoso.fsf@posteo.net> Received-SPF: pass client-ip=198.252.153.129; envelope-from=hanno@hoowl.se; helo=mx1.riseup.net X-Spam_score_int: -25 X-Spam_score: -2.6 X-Spam_bar: -- X-Spam_report: (-2.6 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-Mailman-Approved-At: Sun, 14 Jan 2024 14:30:26 -0500 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:314975 Archived-At: Hej Philip, I have now modified the code according to your suggestions and pushed the changes. Some more detailed responses follow below: > Sure, but first Hanno could you bump the version header, so that the > ELPA build system uses a newer commit, that includes the stylistic > changes + the updated copyright line? Done! > - It seems like a few variable (`jami-bot-account-user-names'?) would be > nice to have as proper user options, defined with `defcustom' and a > type. Good idea! I have implemented that now and the types pass the compilation, so I hope I did that correctly in all instances. > - Try to sharp-quote (#') function symbols. This could just be my limited Elisp-foo, but I could not find any further places in the code where sharp-quotes seem to be appropriate. If this was not meant as a general comment but aimed at a specific bit of code, I would really appreciate a pointer! > - If possible, try to join file paths using functions like > `expand-file-name' with `default-directory' set, instead of using > "low-level" string-manipulation. I looked through the code and could not find an instance where that wasn't already the case. Do you remember where you saw string manipulation being used for path construction? My eyes could simply be too tired to spot that... > - There seem to be some indentation issues (jami-bot-send-message), try > running `indent-region' on the entire buffer. Good catch, fixed! > - There are a few minor checkdoc issues, consider resolving them. Also resolved! Thanks again for the feedback! Best wishes, Hanno