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.abra...@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
