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

Deprecate yielder.prepend #5

Closed

Conversation

JosephTLyons
Copy link
Contributor

@JosephTLyons JosephTLyons commented Dec 12, 2024

I think we might've made a mistake with #1, as we now have yielder.prepend and yielder.append, but yielder.prepend takes in a single element and yielder.prepend .append takes in another yielder of elements. Seems like those functions should have the same inputs, and it is confusing that they don't. My proposal is to rename this to yielder.push_front, to match the same nomenclature used in other gleam libs, such as deque, to avoid confusion.

https://hexdocs.pm/gleam_deque/gleam/deque.html#push_front

(I wouldn't normally open a PR without talking about it first, but this one and #4 are so small that I'm ok with them being throwaways if not wanted).

@JosephTLyons JosephTLyons force-pushed the deprecate-yielder.prepend branch from 27d6980 to 6210b92 Compare December 12, 2024 03:37
@lpil
Copy link
Member

lpil commented Dec 12, 2024

The current design matches the list module, and I don't think we want to have the same function twice with different names, so I think the current implementation is best to stay.

@JosephTLyons
Copy link
Contributor Author

Oh you're right! Maybe that's where I pulled the original PR suggestion from. Whoops! Nothing to see here...

@JosephTLyons JosephTLyons deleted the deprecate-yielder.prepend branch December 13, 2024 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants