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

Fix for exception-unhandled "forked" Promise chain #2101

Merged
merged 3 commits into from
Mar 10, 2025

Conversation

karolyi
Copy link
Contributor

@karolyi karolyi commented Mar 10, 2025

See #2100

I took the liberty of reformatting a bit of code here, I'm sure the autoformatter will have its way with it.

karolyi and others added 2 commits March 10, 2025 13:42
See #2100
for more information, see https://pre-commit.ci
Co-authored-by: Tim Schilling <schilling711@gmail.com>
@karolyi
Copy link
Contributor Author

karolyi commented Mar 10, 2025

I don't find splitting hairs on wording useful, but so be it.

Copy link
Member

@tim-schilling tim-schilling left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me. If someone else can pull it down and confirm it works, we can release it. I'll have time later this week otherwise.

@karolyi
Copy link
Contributor Author

karolyi commented Mar 10, 2025

Does it count when I say this change fixed my original problem outlined? After all, I was the OG PR creator.

@matthiask matthiask merged commit 42696c2 into django-commons:main Mar 10, 2025
25 checks passed
@matthiask
Copy link
Member

I've tested it locally and what worked before still seems to work fine. Will do more tests later. The reason provided in the other PR why writing tests is hard makes sense to me and the comment in the code is sufficient to hopefully be aware of this issue when we have to change the code again in the future, for whatever reason.

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

3 participants