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

Inconsistent Send vs SendMultiple vs NewMessage #69

Open
andig opened this issue Nov 29, 2024 · 4 comments · May be fixed by #70
Open

Inconsistent Send vs SendMultiple vs NewMessage #69

andig opened this issue Nov 29, 2024 · 4 comments · May be fixed by #70
Labels
breaking change enhancement New feature or request help wanted Extra attention is needed

Comments

@andig
Copy link
Contributor

andig commented Nov 29, 2024

While Send takes a Message it returns a *Message. Send uses SendMultiple which works on Message only. Internally, receive works on Messages only. I suggest to change the Send signature to return a Message and modify NewMessage accordingly.

@andig andig linked a pull request Nov 29, 2024 that will close this issue
@andig
Copy link
Contributor Author

andig commented Dec 1, 2024

An alternative approach would of course be to use pointer to message consistently throughout. That would make for simpler error return values (nil instead of Message{}).

@spali
Copy link
Owner

spali commented Dec 1, 2024

Hm... good point.
Still thinking ... to be honest I tend to like pointer everywhere more. Both would be breaking changes, but I assume expect you only the cmd utility is used by others (at least as I know).

@andig
Copy link
Contributor Author

andig commented Dec 1, 2024

I know the difference had caused me quite a bit of confusion. I‘d be happy either way.

@spali spali added enhancement New feature or request help wanted Extra attention is needed breaking change labels Dec 1, 2024
@spali
Copy link
Owner

spali commented Dec 1, 2024

Thanks for the issue and PR... have to find time to do it the pointer way, but doesn't hurry for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants