On  2024-11-08  10:28, Christoph Heiss wrote:
Thanks for the review!

On Thu, Nov 07, 2024 at 04:28:25PM +0100, Aaron Lauterer wrote:
only small nits inline

Tested-By: Aaron Lauterer <a.laute...@proxmox.com>
Reviewed-By: Aaron Lauterer <a.laute...@proxmox.com>

On  2024-10-18  13:59, Christoph Heiss wrote:
[..]
diff --git a/proxmox-fetch-answer/src/fetch_plugins/partition.rs 
b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
index cbfe2d5..c68dc59 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
@@ -9,17 +9,16 @@ use std::{
   static ANSWER_FILE: &str = "answer.toml";
   static ANSWER_MP: &str = "/mnt/answer";
   // FAT can only handle 11 characters, so shorten Automated Installer Source 
to AIS

this comment is now dangling. we could move that to the definition of the
default value for the new parameter

Yep, good catch!

Also, while reading this, seems nowhere is checked whether the
user-supplied value is at max 11 characters long -- I'll add that for v2
too.

Too be honest, do we need to do that? For the default it is important, but users can use other FS that might allow for longer labels and if the kernel of the install live system can handle it, all is good.

[..]
@@ -74,14 +73,14 @@ fn scan_partlabels(partlabel: &str, search_path: &str) -> 
Result<PathBuf> {
       bail!("Could not find partition for label '{partlabel}'");
   }
-/// Will search and mount a partition/FS labeled PARTLABEL (proxmox-ais) in 
lower or uppercase
-/// to ANSWER_MP
-fn mount_proxmoxinst_part() -> Result<String> {
+/// Will search and mount a partition/FS labeled labeled `part_label` in lower 
or uppercase to

one `labeled` too much :)

Thx!



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to