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.

Sign up for GitHub

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

Jump to bottom

Add Dispatcher/Persistence.drop_chat/user_data #2852

Merged
merged 11 commits into from
Jan 19, 2022
Merged

Add Dispatcher/Persistence.drop_chat/user_data #2852

merged 11 commits into from
Jan 19, 2022

Conversation

harshil21
Copy link
Member

@harshil21 harshil21 commented Jan 10, 2022 •
edited
Loading

Closes #2732

Checklist for PRs

  • Added .. versionadded:: version, .. versionchanged:: version or .. deprecated:: version to the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)
  • Created new or adapted existing unit tests

Breaking changes:

  • dp.chat/user_data is now read only
  • BasePersistence.drop_chat/user_data is an abstractmethod

@harshil21 harshil21 added enhancement 📋 pending-review work status: pending-review 🛠 breaking change type: breaking labels Jan 10, 2022
@harshil21 harshil21 added this to the v14 milestone Jan 10, 2022
Bibo-Joshi
Bibo-Joshi reviewed Jan 11, 2022
View reviewed changes
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! didn't have a look at the tests yet.
do you want to add Persistence.drop_*_data is this PR as well?

telegram/ext/_dispatcher.py Outdated Show resolved Hide resolved
telegram/ext/_dispatcher.py Outdated Show resolved Hide resolved
telegram/ext/_dispatcher.py Outdated Show resolved Hide resolved
telegram/ext/_dispatcher.py Outdated Show resolved Hide resolved
telegram/ext/_dispatcher.py Outdated Show resolved Hide resolved
@harshil21 harshil21 removed the 📋 pending-review work status: pending-review label Jan 12, 2022
Bibo-Joshi
Bibo-Joshi reviewed Jan 16, 2022
View reviewed changes
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates!

do you want to add Persistence.drop_*_data is this PR as well?

what about this part?

telegram/ext/_dispatcher.py Outdated Show resolved Hide resolved
telegram/ext/_dispatcher.py Outdated Show resolved Hide resolved
telegram/ext/_dispatcher.py Outdated Show resolved Hide resolved
@harshil21
Copy link
Member Author

do you want to add Persistence.drop_*_data is this PR as well?

what about this part?

Soon, just wanted this review process to be over first. After that is added, then in dp.drop_chat/user_data, we can just call Persistence.drop_chat/user_data instead of dp.update_persistence correct?

@Bibo-Joshi
Copy link
Member

do you want to add Persistence.drop_*_data is this PR as well?

what about this part?

Soon, just wanted this review process to be over first. After that is added, then in dp.drop_chat/user_data, we can just call Persistence.drop_chat/user_data instead of dp.update_persistence correct?

jup 👍

@harshil21 harshil21 changed the title Add Dispatcher.drop_chat/user_data Add Dispatcher/Persistence.drop_chat/user_data Jan 17, 2022
Bibo-Joshi
Bibo-Joshi reviewed Jan 18, 2022
View reviewed changes
telegram/ext/_dictpersistence.py Outdated Show resolved Hide resolved
telegram/ext/_dictpersistence.py Outdated Show resolved Hide resolved
telegram/ext/_dispatcher.py Outdated Show resolved Hide resolved
telegram/ext/_picklepersistence.py Outdated Show resolved Hide resolved
telegram/ext/_picklepersistence.py Outdated Show resolved Hide resolved
telegram/ext/_dispatcher.py Outdated Show resolved Hide resolved
telegram/ext/_dispatcher.py Outdated Show resolved Hide resolved
tests/test_dispatcher.py Outdated Show resolved Hide resolved
@harshil21
Review 3
05bc8b0 
Update a ValueError message
Bibo-Joshi
Bibo-Joshi reviewed Jan 19, 2022
View reviewed changes
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates ❤️ Code LGTM! I'll merge v14 with the animation-test fix so that we can have another go ad the CI - maybe codecov also shows up again …

@harshil21
Copy link
Member Author

Code LGTM

🎉
I had already merged v14 with animation test fix. They just changed the animation values again...

@Bibo-Joshi Bibo-Joshi merged commit b1dcc93 into v14 Jan 19, 2022
@Bibo-Joshi Bibo-Joshi deleted the drop_chat_user_data branch January 19, 2022 16:27
@Bibo-Joshi Bibo-Joshi mentioned this pull request Jan 19, 2022
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Jan 26, 2022
@Bibo-Joshi Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Poolitzer Poolitzer Poolitzer left review comments

@Bibo-Joshi Bibo-Joshi Bibo-Joshi left review comments

Assignees
No one assigned
Labels
🛠 breaking change type: breaking 🔌 enhancement pr description: enhancement
Projects
None yet
Milestone
v20.0a0
Development

Successfully merging this pull request may close these issues.
3 participants
@harshil21 @Bibo-Joshi @Poolitzer