<div dir="ltr"><br>> This commit is really hard to follow.  I wonder if it can be broken up<br>> to make it easier to review.  There seem to be at least two parts to<br>> the patch: (1) Changing the way that we detect if a device contains<br>> partitions.  (2) Changing the way that we filter out LDM partitions.<br><br>Agree. Splitted this commit to a series of more granular commits.<br><br><div>> "check_device", "check_partition" are really generic names.  Name the<br>> functions according to what they do, eg.  "is_not_partitioned_device".<br><br></div><div>Agree.</div><div><br></div><div>> > +and is_ignored_gpt_type gpt_type =<br>> > +  match gpt_type with<br>><br>> You can write this as:<br>><br>>   and is_ignored_gpt_type = function<br>>   | pattern -> result<br>>   | ...<br><br>I've got rid of separate function at all.<br><br>> > +  (* Windows Logical Disk Manager metadata partition. *)<br>> > +  | "5808C8AA-7E8F-42E0-85D2-E1E90434CFB3" -> Optgroups.ldm_available ()<br>> > +  (* Windows Logical Disk Manager data partition. *)<br>> > +  | "AF9B60A0-1431-4F62-BC68-3311714A69AD" -> Optgroups.ldm_available ()<br>><br>> Why does the result depend on Optgroups.ldm_available ()?<br><br>Yes, it shouldn't.<br><br>> > +  (* Microsoft Reserved Partition. *)<br>> > +  | "E3C9E316-0B5C-4DB8-817D-F92DF00215AE" -> true<br>> > +  (* Windows Snapshot Partition. *)<br>> > +  | "CADDEBF1-4400-4DE8-B103-12117DCF3CCF" -> true<br>><br>> You can combine the right hand sides of multiple matches, which helps<br>> the pattern match optimizer in the compiler, eg:<br>><br>>   (* Microsoft Reserved Partition. *)<br>>   | "E3C9E316-0B5C-4DB8-817D-F92DF00215AE"<br>>   (* Windows Snapshot Partition. *)<br>>   | "CADDEBF1-4400-4DE8-B103-12117DCF3CCF" -> true<br>>   | _ -> false<br><br>Ah, thanks.</div><div><br>> Catching the generic exception is usually a big red flag.  What<br>> exceptions are you expecting here?  If you're not expecting an<br>> exception, don't try to catch anything.<br></div><div><br></div><div>Agree. There is no expected exceptions there.</div><div>I thought: If something went wrong - let's not filter out device or partition.</div><div><br></div><div>v6 is coming.</div><div><br></div><div>--</div><div>  Mykola Ivanets</div></div>