allan2 / dotenvy

A well-maintained fork of the dotenv crate
MIT License
664 stars 40 forks source link

`Iter` type being leaked publicly #48

Open sonro opened 1 year ago

sonro commented 1 year ago

The public functions dotenv_iter, from_read_iter, from_filename_iter, and from_path_iter all return the Iter struct - which is currently not public. It needs to be so users have easier to access its documentation.

As a public type it needs to eagerly implement common traits (Rust API Guidelines) and have decent documentation.

Its naming could also be improved. It is clear enough in the context of a library internal, but when constructed and viewed externally Iter is fairly nebulous. Are there any renaming suggestions?

Making Iter public would also make from_read_iter obsolete as all it does is call Iter::new.

LeoniePhiline commented 1 year ago

Naming: Iter is fine, since it is namespaced by the crate. See also https://docs.rs/error-chain/latest/error_chain/struct.Iter.html

There is only one kind of Iter in this crate, and the crate focuses on a single functionality.

allan2 commented 1 year ago

Making Iter public sounds good to me.

I would keep from_read_iter around, at least for the time being.

LeoniePhiline commented 1 year ago

So pub use iter::Iter in the root namespace? (I'd rather not pub mod iter, as the nested namespace is useless to library users.)

sonro commented 1 year ago

I am leaving this issue unresolved until the documentation for the type has been written. Thank you @LeoniePhiline for the starter PR.

allan2 commented 4 days ago

On the v0.16 branch, I have made Iter private. The *_iter functions are now deprecated. EnvLoader::load returns HashMap.

Does anyone object to Iter being private?