On 25.08.2025 16:37, Wolfgang Bumiller wrote:
On Mon, Aug 25, 2025 at 01:39:48PM +0200, Gabriel Goller wrote:
On 25.08.2025 10:11, Wolfgang Bumiller wrote:
> > diff --git a/pve-rs/src/bindings/sdn/fabrics.rs 
b/pve-rs/src/bindings/sdn/fabrics.rs
> > index 1dc8bf4320e6..3f70d421e582 100644
> > --- a/pve-rs/src/bindings/sdn/fabrics.rs
> > +++ b/pve-rs/src/bindings/sdn/fabrics.rs
> > @@ -9,8 +9,10 @@ pub mod pve_rs_sdn_fabrics {
> >      use std::fmt::Write;
> >      use std::net::IpAddr;
> >      use std::ops::Deref;
> > +    use std::process::Command;
> >      use std::sync::Mutex;
> >
> > +    use anyhow::Context;
> >      use anyhow::Error;
> >      use openssl::hash::{MessageDigest, hash};
> >      use serde::{Deserialize, Serialize};
> > @@ -578,4 +580,196 @@ pub mod pve_rs_sdn_fabrics {
> >
> >          Ok(interfaces)
> >      }
> > +
> > +    /// This module contains status-related structs that represent Routes 
and Neighbors for all
> > +    /// protocols
> > +    pub mod status {
>
> ^ This seems to be a helper module which does not contain any
> perlmod/perl specifics.
> I'd argue it's time to start a `crate::sdn` module outside of the
> `bindings` submodule for this.
>
> The `bindings` module should become rather more lean in the future and
> focus mostly on the perl/rust interaction.

Umm do I understand you correctly that you want to have something like
this:

    src/
    ├─ bindings/
    │  ├─ sdn/
    │  │  ├─ fabrics.rs
    ├─ sdn/
    │  ├─ status.rs

?

Yes. The bindings should just be the perl interface and its point is to
provide documentation via rustdoc, and the rustdocs should tell you how
to use it *from perl*.

The rest would be additional code we need to provide the perl interface
for the external crates.

IMO we could move all the status stuff out to
crate::bindings::sdn::status. But I don't know about separating all the
types, conversion methods and actual perl methods -- I'd rather keep all
the perl-facing stuff in the same file.

You don't need to separate everything. My point was that it does *not*
contain perl *binding* specifics. As for being *perl* specific, I mean,
the entire `pve-rs` crate *is* perl specific right now...

True :)
Should we maybe have a 'types' module in the middle, so that we have:

src/bindings/sdn/fabrics.rs
src/types/sdn/fabrics.rs

?



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

Reply via email to