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

EDU-2392: Removing repetitive code snippet #3246

Merged
merged 3 commits into from
Jan 10, 2025

Conversation

jsundai
Copy link
Contributor

@jsundai jsundai commented Dec 18, 2024

What does this PR do?

Notes to reviewers

@jsundai
Copy link
Contributor Author

jsundai commented Dec 18, 2024

@brianmacdonald-temporal Ready for review

@fairlydurable
Copy link
Contributor

I don't see any problems but be aware that with at least one of these patches, there was a subtle but important difference between the two code snippets that seemed to be redundant at first and, in fact, needed clarification as to what each one did.

I'm cc'ing in @axfelix for a second pair of eyes

@axfelix
Copy link
Contributor

axfelix commented Jan 10, 2025

This one is actually redundant, though it's a bit unintuitive now, it should really segue directly into this part https://docs.temporal.io/develop/python/versioning#deprecated-patches to show an example of deprecate_patch().

@fairlydurable
Copy link
Contributor

Thanks for checking @axfelix

Going to approve and unblock.

Copy link
Contributor

@fairlydurable fairlydurable left a comment

Choose a reason for hiding this comment

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

We're good to go. Unblocking.

@brianmacdonald-temporal brianmacdonald-temporal merged commit 11954e8 into main Jan 10, 2025
3 checks passed
@brianmacdonald-temporal brianmacdonald-temporal deleted the edu-2392-code-snippet branch January 10, 2025 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants