[pbs-devel] [PATCH proxmox v5 1/2] fix #5379: api-types: add `default` field for all realm types

[Christoph Heiss]

Both good catches, thanks!
I'll fix them up for the next revision.

On Fri Mar 21, 2025 at 5:04 PM CET, Shannon Sterz wrote:
> On Fri Mar 21, 2025 at 2:45 PM CET, Christoph Heiss wrote:
[..]
>> diff --git a/pbs-api-types/src/ad.rs b/pbs-api-types/src/ad.rs
>> index 910571a0..5c236325 100644
>> --- a/pbs-api-types/src/ad.rs
>> +++ b/pbs-api-types/src/ad.rs
>> @@ -64,6 +68,9 @@ pub struct AdRealmConfig {
>>      /// Comment
>>      #[serde(skip_serializing_if = "Option::is_none")]
>>      pub comment: Option<String>,
>> +    /// True if it should be the default realm to login in
>
> tiniest of nit: "realm to login in" sounds wrong. I think you could just
> remove everything after realm.

Yeah, sounds good, I'll change it to that!

>
>> +    #[serde(skip_serializing_if = "Option::is_none")]
>> +    pub default: Option<bool>,
>>      /// Connection security
>>      #[serde(skip_serializing_if = "Option::is_none")]
>>      pub mode: Option<LdapMode>,
[..]
>> diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
>> index acc2fca3..e3896000 100644
>> --- a/pbs-api-types/src/lib.rs
>> +++ b/pbs-api-types/src/lib.rs
[..]
>>  #[derive(Deserialize, Serialize, Clone, PartialEq)]
>>  #[serde(rename_all = "kebab-case")]
>>  /// Basic Information about a realm
>>  pub struct BasicRealmInfo {
>> +    /// Realm name
>>      pub realm: String,
>> +    // Realm type
>
> nit: this is not a doc-comment, just a regular one
>
>>      #[serde(rename = "type")]
>>      pub ty: RealmType,
>>      /// True if it is the default realm
>>      #[serde(skip_serializing_if = "Option::is_none")]
>>      pub default: Option<bool>,
>> +    /// Optional comment for this realm
>>      #[serde(skip_serializing_if = "Option::is_none")]
>>      pub comment: Option<String>,
>>  }