Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Iter type being leaked publicly #48

Closed
sonro opened this issue Dec 6, 2022 · 5 comments · Fixed by #123
Closed

Iter type being leaked publicly #48

sonro opened this issue Dec 6, 2022 · 5 comments · Fixed by #123
Labels
documentation Improvements or additions to documentation

Comments

@sonro
Copy link
Collaborator

sonro commented Dec 6, 2022

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.

@sonro sonro added the bug Something isn't working label Dec 6, 2022
@sonro sonro changed the title Iter type being leaked publicly. Iter type being leaked publicly Dec 6, 2022
@LeoniePhiline
Copy link
Contributor

LeoniePhiline commented Dec 7, 2022

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
Copy link
Owner

allan2 commented Dec 10, 2022

Making Iter public sounds good to me.

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

@LeoniePhiline
Copy link
Contributor

LeoniePhiline commented Dec 11, 2022

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
Copy link
Collaborator Author

sonro commented Dec 14, 2022

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

@sonro sonro added documentation Improvements or additions to documentation and removed bug Something isn't working labels Dec 14, 2022
@allan2
Copy link
Owner

allan2 commented Sep 4, 2024

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?

@allan2 allan2 mentioned this issue Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants