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

Issue-981: Improve narrative for chatId. #1305

Merged
merged 1 commit into from
Feb 23, 2023
Merged

Conversation

manstis
Copy link

@manstis manstis commented Feb 23, 2023

See #981

The consensus was that the documentation wasn't entirely clear as to whether chatId was mandatory or not.

It is mandatory but not necessarily as a Configuration Option if provided in the header.

@oscerd oscerd modified the milestones: 3.20.2, 4.0.0 Feb 23, 2023
@oscerd
Copy link
Contributor

oscerd commented Feb 23, 2023

Needs to be backported on 3.x and 3.20.x too.

@manstis
Copy link
Author

manstis commented Feb 28, 2023

@oscerd I've been thinking on this...

This PR made chat Id mandatory which is a breaking change (and is the cause of my re-consideration).

I'm thinking it may have been better to simply improve the description to read along the lines of chat Id being required as either a configuration option or supplied in the message header without making it explicitly required.

WDYT? Happy to submit more PRs.

@oscerd
Copy link
Contributor

oscerd commented Feb 28, 2023

This is on main branch, so a breaking change is fine. I'm happy to merge more stuff if you feel it would be better to just add a description

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.

None yet

2 participants