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

#893 Problems to cancel a request (CancellationToken) #1367

Merged
merged 17 commits into from
Sep 23, 2023
Merged

#893 Problems to cancel a request (CancellationToken) #1367

merged 17 commits into from
Sep 23, 2023

Conversation

jlukawska
Copy link
Contributor

@jlukawska jlukawska commented Nov 10, 2020 •
edited by raman-m

Fixes #893

Proposed Changes

  • One more problem related to HttpContext: cancellation token was not copied to the new HttpContext.

@nicot4e
Copy link

nicot4e commented Mar 28, 2023

Please merge this pull request! I just confirmed that it still is a fully functioning fix for #893
What are we waiting for? 🙂

@starx207
Copy link

I second @nicot4e. I ended up having to fork the repo and make this change myself - it was a showstopper issue for us

@nicot4e nicot4e mentioned this pull request Mar 28, 2023
Closed
@amaleszewski
Copy link

@TomPallister bump. please merge that pull request - it's a no-go for many of us!

@amaleszewski amaleszewski mentioned this pull request Apr 13, 2023
Closed
amaleszewski
amaleszewski previously approved these changes Apr 14, 2023
View reviewed changes
@dvandenhoek
Copy link

@raman-m Please review and complete this PR. This is a big issue for alot of ppl and a simple fix

@raman-m
Copy link
Member

raman-m commented May 16, 2023

Hi @jlukawska !

Requesting to master is wrong!
You can request PRs to develop branch only!

Please, change the base (target) branch!
I would recommend to pull latest commits from base:develop and apply your changes on the top commit!

Pay attention that a release can be made by Tom only mastering develop branch! There is no sense to request to other branches!

@raman-m
Copy link
Member

raman-m commented May 16, 2023 •
edited

@dvandenhoek commented on May 16, 2023

I'll review as soon as the issue 893 will be reviewed and accepted.

@jlukawska
Copy link
Contributor Author

jlukawska commented May 16, 2023 •
edited by raman-m

@raman-m commented on May 16, 2023

It's not polite to yell at people...

In 2020 (almost 3 years ago) when I created this PR, all pull requests were created to the master branch, not develop.
@TomPallister required it, see: #1074 (comment)

@raman-m
Copy link
Member

raman-m commented May 16, 2023

Hi @TomPallister
Could you resolve this management issue please?

@ThreeMammals ThreeMammals locked and limited conversation to collaborators May 16, 2023
@WeihanLi WeihanLi mentioned this pull request May 18, 2023
Merged
@raman-m raman-m changed the title Fix/893 cancel request #893 Problems to cancel a request (CancellationToken) May 19, 2023
@raman-m raman-m changed the base branch from master to develop May 19, 2023 13:59
@raman-m raman-m dismissed amaleszewski’s stale review May 19, 2023 13:59

The base branch was changed.

@raman-m raman-m added bug Identified as a potential bug feature A new feature small effort Likely less than a day of development effort. labels May 19, 2023
@raman-m raman-m self-requested a review May 19, 2023 14:04
raman-m
raman-m previously approved these changes May 20, 2023
View reviewed changes
Copy link
Member

@raman-m raman-m left a comment

Choose a reason for hiding this comment

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

@jlukawska
Congrats! We are on the green road!

And, thanks for this micro elegant fix!

@raman-m raman-m added the accepted Bug or feature would be accepted as a PR or is being worked on label May 20, 2023
@ThreeMammals ThreeMammals unlocked this conversation May 20, 2023
@raman-m raman-m requested a review from TomPallister May 20, 2023 13:33
raman-m
raman-m previously approved these changes Jul 15, 2023
View reviewed changes
@raman-m
Copy link
Member

raman-m commented Jul 15, 2023

@jlukawska Hey J!
Could you Sync fork please? So, your develop branch is outdated!

@raman-m raman-m mentioned this pull request Jul 19, 2023
Closed
RaynaldM
RaynaldM previously approved these changes Aug 11, 2023
View reviewed changes
WeihanLi
WeihanLi approved these changes Aug 11, 2023
View reviewed changes
Copy link
Contributor

@WeihanLi WeihanLi left a comment

Choose a reason for hiding this comment

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

LGTM

@dvandenhoek dvandenhoek mentioned this pull request Sep 4, 2023
Closed
@raman-m raman-m mentioned this pull request Sep 6, 2023
Open
@voltcode
Copy link

voltcode commented Sep 6, 2023

Hi, what's missing to merge this ? can we ping the missing approver ?

raman-m
raman-m previously approved these changes Sep 6, 2023
View reviewed changes
Copy link
Member

@raman-m raman-m left a comment •
edited

Choose a reason for hiding this comment

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

+ Approved

@raman-m raman-m added accepted Bug or feature would be accepted as a PR or is being worked on and removed needs feedback Issue is waiting on feedback before acceptance labels Sep 6, 2023
@raman-m
Copy link
Member

raman-m commented Sep 6, 2023

@voltcode commented on Sep 6, 2023

Don't worry! 😉
The PR has been approved and accepted finally!
But, Tom has the last word, because he has the right to merge.

@raman-m
Copy link
Member

raman-m commented Sep 6, 2023

@TomPallister
The community would love to see this fix in the next release.
Could you merge it please?

@raman-m raman-m requested review from TomPallister and removed request for amaleszewski September 6, 2023 14:55
@MuruAE
Copy link

MuruAE commented Sep 18, 2023

Hi @raman-m @TomPallister
I want to express our sincere appreciation for the valuable contributions you are making to the community. We have a strong dependency on the Cancellation Token change for an upcoming delivery, and all the discussions so far have been quite promising, indicating that this change is approaching the next release.

To better align our plans, I would like to inquire about the expected release timeline for this change. If the release is anticipated in the near future, we are more than willing to wait. However, if there's a significant delay expected, we may consider forking the repository to address our immediate needs.

Thank you for your continued support and collaboration.

@raman-m
Copy link
Member

raman-m commented Sep 18, 2023 •
edited

@MuruAE commented on Sep 18

If you have short delivery timeline, then I would suggest to make a build from current feature branch and reference DLL with the bug fix via project reference, not NuGet package one!

Update 1

Next release date is October 1...

@MuruAE
Copy link

MuruAE commented Sep 21, 2023 •
edited by raman-m

@raman-m commented on Sep 18

Thanks @raman-m really appreciated for the quick turnaround.

@raman-m raman-m dismissed stale reviews from RaynaldM and themself via 13ec519 September 23, 2023 09:08
@raman-m raman-m merged commit 5ad807d into ThreeMammals:develop Sep 23, 2023
2 checks passed
@raman-m
Copy link
Member

raman-m commented Sep 23, 2023

@MuruAE @voltcode @WeihanLi @dvandenhoek @amaleszewski @starx207 @nicot4e @jlukawska
Hi Oceloters! 😺
I know, you have been waiting for this bug fix for 3 years. I am planning to release this fix on October 1...

@raman-m raman-m mentioned this pull request Oct 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WeihanLi WeihanLi WeihanLi approved these changes

@raman-m raman-m raman-m approved these changes

@RaynaldM RaynaldM RaynaldM left review comments

@amaleszewski amaleszewski amaleszewski left review comments

@binarymash binarymash Awaiting requested review from binarymash

@jps jps Awaiting requested review from jps

@geffzhang geffzhang Awaiting requested review from geffzhang

@philproctor philproctor Awaiting requested review from philproctor

@anktsrkr anktsrkr Awaiting requested review from anktsrkr

@sharpn sharpn Awaiting requested review from sharpn

@abooraja abooraja Awaiting requested review from abooraja

@wast wast Awaiting requested review from wast

@yangzhongke yangzhongke Awaiting requested review from yangzhongke

@thiagoloureiro thiagoloureiro Awaiting requested review from thiagoloureiro

@TomPallister TomPallister Awaiting requested review from TomPallister

Assignees

@jlukawska jlukawska

Labels
accepted Bug or feature would be accepted as a PR or is being worked on bug Identified as a potential bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Problems to cancel a request (CancellationToken)
10 participants
@jlukawska @nicot4e @starx207 @amaleszewski @dvandenhoek @raman-m @voltcode @MuruAE @RaynaldM @WeihanLi