From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id 17A581FF16F
for <inbox@lore.proxmox.com>; Thu, 13 Mar 2025 10:06:41 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 44C0F1401E;
Thu, 13 Mar 2025 10:06:31 +0100 (CET)
Message-ID: <f476cdcc-be94-4fcc-ae26-6f7ac9d81b92@proxmox.com>
Date: Thu, 13 Mar 2025 10:05:56 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Alexander Abraham <a.abraham@proxmox.com>
References: <20250311115200.50172-1-a.abraham@proxmox.com>
Content-Language: en-US
From: Friedrich Weber <f.weber@proxmox.com>
In-Reply-To: <20250311115200.50172-1-a.abraham@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.142 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
POISEN_SPAM_PILL 0.1 Meta: its spam
POISEN_SPAM_PILL_2 0.1 random spam to be learned in bayes
POISEN_SPAM_PILL_4 0.1 random spam to be learned in bayes
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH pve_flutter_frontend] Fix #6231: Removed
whitespace-trimming from password entry field
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
Hi, thanks for the patch! I can't comment on the change itself, but have
some remarks on the commit message:
On 11/03/2025 12:52, Alexander Abraham wrote:
> A user reported a bug where they were attempting to login into our
> app for PVE and they used a password with two spaces at the end.
> The login failed because of these trailing spaces. I removed the
Note that commit messages should be written in imperative mood, see [1]
-- so the "I removed" should be rephrased to something like "Remove the
string trimming [...]".
Same for the first line of the comment message ("Removed").
If possible, it is also be nice to have a tag prefix in the first line
that describes the involved subsystem (as described in [1]). What I
usually do to get an idea is to check previous commits that touch the
same file(s) -- proxmox_login_form.dart (via `git log
proxmox_login_form.dart` or the git UI [2]) in your case. Here we're
lucky and there are several commits that use "login:" or "login form:",
[2] so either of them sound good. But of course this approach doesn't
always work, so sometimes I'll have to come up with something on my own.
Finally, and I realize this is not in [1] so we're entering personal
preference territory, but I still want to mention it: Most first lines
of commit messages in our projects start in lowercase, so you can
consider adapting that convention too.
So all in all, a nice first line might be
fix #6231: login form: remove whitespace trimming from password entry
or something like that.
I realize these suggestions may sound a little pedantic :) But in the
long run, sticking to the guidelines makes maintenance of our projects
easier. For example, if all commit messages are formulated in similar
style, one can skim commit logs faster when looking for bugs or new feature.
One more thing: When you send an initial patch (series) for a bug
report, it can also be nice to drop a (lore.proxmox.com) link in the bug
report for reference, so users can take a look at the series if they want.
> string trimming for the password entry field during login so users can
> use passwords that have spaces inside them.
>
> Signed-off-by: Alexander Abraham <a.abraham@proxmox.com>
> ---
> lib/proxmox_login_form.dart | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/proxmox_login_form.dart b/lib/proxmox_login_form.dart
> index 735bd42..5916563 100644
> --- a/lib/proxmox_login_form.dart
> +++ b/lib/proxmox_login_form.dart
> @@ -464,7 +464,7 @@ class _ProxmoxLoginPageState extends State<ProxmoxLoginPage> {
> //cleaned form fields
> final origin = normalizeUrl(_originController.text.trim());
> final username = _usernameController.text.trim();
> - final String enteredPassword = _passwordController.text.trim();
> + final String enteredPassword = _passwordController.text;
> final String? savedPassword = widget.password;
>
> final password = ticket.isNotEmpty ? ticket : enteredPassword;
[1]
https://pve.proxmox.com/wiki/Developer_Documentation#Commits_and_Commit_Messages
[2]
https://git.proxmox.com/?p=flutter/proxmox_login_manager.git;a=history;f=lib/proxmox_login_form.dart;h=735bd42d7c493aa8e39177b28963db00c6c29ebc;hb=HEAD
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel