All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: "Linus Walleij" <linus.walleij@linaro.org>,
	"Bartosz Golaszewski" <brgl@bgdev.pl>,
	"Vincent Guittot" <vincent.guittot@linaro.org>,
	linux-gpio@vger.kernel.org,
	"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com>,
	"Wedson Almeida Filho" <wedsonaf@google.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	stratos-dev@op-lists.linaro.org,
	"Gerard Ryan" <g.m0n3y.2503@gmail.com>
Subject: Re: [PATCH V4 4/8] libgpiod: Add rust wrapper crate
Date: Mon, 1 Aug 2022 21:20:17 +0800	[thread overview]
Message-ID: <20220801132017.GA46663@sol> (raw)
In-Reply-To: <20220801120506.46emxdk7qk2g2gmf@vireshk-i7>

On Mon, Aug 01, 2022 at 05:35:06PM +0530, Viresh Kumar wrote:
> On 27-07-22, 18:08, Kent Gibson wrote:
> > On Wed, Jul 27, 2022 at 02:37:01PM +0530, Viresh Kumar wrote:
> > > On 27-07-22, 10:57, Kent Gibson wrote:
> > > > On Fri, Jul 08, 2022 at 05:04:57PM +0530, Viresh Kumar wrote:
> > > > Consider modules and namespaces rather than lumping everything in
> > > > the gpiod space.
> > > > 
> > > > e.g. gpiod::ChipInfo -> gpiod::chip::Info
> > > 
> > > The modules are already there, as file names. So what we really have is
> > > gpiod::chip_info::ChipInfo (yeah it isn't great for sure). But then it looked
> > > tougher/complex/unnecessary for end users to know the internals of a dependency
> > > and so I did this:
> > > 
> > > pub use crate::chip::*;
> > > pub use crate::edge_event::*;
> > > pub use crate::event_buffer::*;
> > > pub use crate::info_event::*;
> > > pub use crate::line_config::*;
> > > pub use crate::line_info::*;
> > > pub use crate::line_request::*;
> > > pub use crate::request_config::*;
> > > 
> > > which puts everything under gpiod::*. I think it is easier for users that way.
> > > The modules are fine for in-crate management, but for end user they shouldn't be
> > > visible.
> > > 
> > 
> > The main problem I have with putting everything in the top level is the
> > generated docs.  You get everything dumped on you, so all structs, enums
> > and functions, and it isn't clear to the user what the logical starting
> > point is.
> > If things are tiered then you can introduce them more gradually, or keep
> > them out of their way if they are unlikely to use them (e.g. ChipInfo,
> > InfoEvent).
> 
> I am not sure what to do about this. I was suggested earlier to dump it all at
> the top level namespace so it is easier/shorter for users, which also won't need
> to know the internal namespaces/modules of libgpiod.
> 

libgpiod has no namespaces/modules - cos it is C.
The users do need to understand how things fit together, and the flat
namespace doesn't help there - everything is equal.

True, "_" is shorter than "::".  Can't argue with that ;-).

In Rust the user can rename the import whatever they want, so I'm not sure
the "easier/shorter" argument holds much water.

> Looking at structures, there are just 8 of them which are exposed in docs now,
> which isn't a lot really. And then we have 12 enums now.
> 

This isn't a question of numbers, it is a question of whether to
indicate structure, or to flatten everything.

I find the structured approach both clearer and more idiomatic in Rust,
so that gets my vote.

Cheers,
Kent.

  reply	other threads:[~2022-08-01 13:20 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-08 11:34 [PATCH V4 0/8] libgpiod: Add Rust bindings Viresh Kumar
2022-07-08 11:34 ` [PATCH V4 1/8] libgpiod: Add libgpiod-sys rust crate Viresh Kumar
2022-07-27  2:57   ` Kent Gibson
2022-07-27  4:51     ` Viresh Kumar
2022-07-27  5:17       ` Kent Gibson
2022-07-27  5:45         ` Viresh Kumar
2022-08-01 12:11         ` Viresh Kumar
2022-08-01 15:56           ` Kent Gibson
2022-08-02  8:50             ` Viresh Kumar
2022-08-02  9:36               ` Kent Gibson
2022-07-08 11:34 ` [PATCH V4 2/8] libgpiod: Add pre generated rust bindings Viresh Kumar
2022-07-27  2:57   ` Kent Gibson
2022-07-27  5:15     ` Viresh Kumar
2022-07-27  5:31       ` Kent Gibson
2022-07-27  6:00         ` Viresh Kumar
2022-07-27  6:06           ` Kent Gibson
2022-07-08 11:34 ` [PATCH V4 3/8] libgpiod-sys: Add support to generate gpiosim bindings Viresh Kumar
2022-07-27  2:57   ` Kent Gibson
2022-07-27  5:30     ` Viresh Kumar
2022-07-08 11:34 ` [PATCH V4 4/8] libgpiod: Add rust wrapper crate Viresh Kumar
2022-07-27  2:57   ` Kent Gibson
2022-07-27  9:07     ` Viresh Kumar
2022-07-27 10:08       ` Kent Gibson
2022-07-27 11:06         ` Miguel Ojeda
2022-07-27 12:40           ` Kent Gibson
2022-07-27 13:02             ` Miguel Ojeda
2022-07-28  3:11               ` Kent Gibson
2022-07-29  4:40                 ` Viresh Kumar
2022-07-28  3:10         ` Kent Gibson
2022-08-01 12:05         ` Viresh Kumar
2022-08-01 13:20           ` Kent Gibson [this message]
2022-08-01 13:28             ` Miguel Ojeda
2022-07-28  8:52     ` Viresh Kumar
2022-07-28  9:59       ` Kent Gibson
2022-07-08 11:34 ` [PATCH V4 5/8] libgpiod: Add rust examples Viresh Kumar
2022-07-27  2:58   ` Kent Gibson
2022-07-27  9:23     ` Viresh Kumar
2022-07-27  9:59       ` Kent Gibson
2022-07-27 10:06         ` Viresh Kumar
2022-07-27 10:32           ` Kent Gibson
2022-07-27 10:33             ` Viresh Kumar
2022-07-08 11:34 ` [PATCH V4 6/8] libgpiod: Derive debug traits for few definitions Viresh Kumar
2022-07-27  2:58   ` Kent Gibson
2022-07-27  6:20     ` Viresh Kumar
2022-07-08 11:35 ` [PATCH V4 7/8] libgpiod: Add rust tests Viresh Kumar
2022-07-27  2:58   ` Kent Gibson
2022-07-27  9:59     ` Viresh Kumar
2022-07-27 10:27       ` Kent Gibson
2022-08-01 11:54     ` Viresh Kumar
2022-08-01 12:38       ` Kent Gibson
2022-08-02  5:44         ` Viresh Kumar
2022-08-02  5:47           ` Kent Gibson
2022-07-08 11:35 ` [PATCH V4 8/8] libgpiod: Integrate building of rust bindings with make Viresh Kumar
2022-07-27  2:59   ` Kent Gibson
2022-07-27  6:18     ` Viresh Kumar
2022-07-27  6:25       ` Kent Gibson
2022-07-27  6:35         ` Viresh Kumar
2022-07-27  6:45           ` Kent Gibson
2022-07-27  6:51             ` Viresh Kumar
2022-07-15 19:07 ` [PATCH V4 0/8] libgpiod: Add Rust bindings Bartosz Golaszewski
2022-07-15 19:17   ` Miguel Ojeda
2022-07-15 19:27     ` Miguel Ojeda
2022-07-16  9:43       ` Miguel Ojeda
2022-07-16 10:43         ` Bartosz Golaszewski
2022-07-16 12:23           ` Kent Gibson
2022-07-16 13:46           ` Miguel Ojeda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220801132017.GA46663@sol \
    --to=warthog618@gmail.com \
    --cc=alex.bennee@linaro.org \
    --cc=brgl@bgdev.pl \
    --cc=g.m0n3y.2503@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=stratos-dev@op-lists.linaro.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.org \
    --cc=wedsonaf@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.