Skip to content

Conversation

@alex-inftx
Copy link
Contributor

we can dispose CancellationTokenSource after use

@ghost ghost added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-runtime community-contribution Indicates that the PR has been added by a community member labels Aug 2, 2023
@ghost
Copy link

ghost commented Aug 2, 2023

Thanks for your PR, @alex-inftx. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@ghost ghost added this to the 6.0.x milestone Aug 2, 2023
@ghost
Copy link

ghost commented Aug 2, 2023

Hi @alex-inftx. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@gfoidl
Copy link
Member

gfoidl commented Aug 2, 2023

@alex-inftx please change the target branch of this PR to be main (and not release/6.0).

@sharwell sharwell changed the base branch from release/6.0 to main August 2, 2023 12:39
@sharwell sharwell changed the base branch from main to release/6.0 August 2, 2023 12:40
@sharwell
Copy link
Contributor

sharwell commented Aug 2, 2023

I changed the base branch to main not realizing that release/6.0 hasn't merged into main yet. Sorry for the noise.

@mgravell
Copy link
Contributor

mgravell commented Aug 2, 2023

I changed the base branch to main not realizing that release/6.0 hasn't merged into main yet. Sorry for the noise.

Still looks like 6.0 unless I'm misreading; if you click edit, the target branch should become editable:

image

However, if it starts freaking out, it may be easier to redo the branch against main.

as an aside: there is no "yet" here - 6.0 is a maintenance branch; it'll stay independent until some future hypothetical point when it simply gets nuked

@sebastienros sebastienros changed the base branch from release/6.0 to main August 2, 2023 15:20
@sebastienros
Copy link
Member

I changed the base branch but now there are lots of conflicts. Might be easier to create a new PR.

@sebastienros sebastienros changed the base branch from main to release/6.0 August 2, 2023 15:21
@mgravell
Copy link
Contributor

mgravell commented Aug 2, 2023

image

yeah, that would be the "freaking out" I was hoping to avoid :) in theory it should be possible to rebase and force push, but for a 2 line change, starting a clean branch from main is probably much much simpler

@MichaelSimons MichaelSimons removed the request for review from a team August 2, 2023 15:52
@alex-inftx alex-inftx changed the base branch from release/6.0 to main August 2, 2023 16:04
@alex-inftx alex-inftx closed this Aug 2, 2023
@alex-inftx alex-inftx reopened this Aug 2, 2023
@ghost
Copy link

ghost commented Aug 2, 2023

Hi @alex-inftx. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@alex-inftx
Copy link
Contributor Author

@alex-inftx please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@dotnet-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree company="infotecs"

@alex-inftx
Copy link
Contributor Author

image

yeah, that would be the "freaking out" I was hoping to avoid :) in theory it should be possible to rebase and force push, but for a 2 line change, starting a clean branch from main is probably much much simpler

It's done [rebase and+force push] )

Copy link
Contributor

@mgravell mgravell 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 good to me, and should clean up a timer callback 👍

@alex-inftx
Copy link
Contributor Author

This looks good to me, and should clean up a timer callback 👍
thank you!
@mgravell , should I create a new PRs to place changes to release/6.0 and release/7.0 ?

@mgravell
Copy link
Contributor

mgravell commented Aug 2, 2023

@alex-inftx IMO no this doesn't warrant backport. Yes, it may be more efficient, but: the feature isn't fundamentally broken or dangerous "as is". Others may disagree, but I vote "no"

@mkArtakMSFT mkArtakMSFT removed the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Aug 2, 2023
@adityamandaleeka adityamandaleeka merged commit 22b8ebb into dotnet:main Aug 2, 2023
@adityamandaleeka
Copy link
Member

Thanks @alex-inftx. I agree with @mgravell that this doesn't need to be backported.

@ghost ghost modified the milestones: 6.0.x, 8.0-rc1 Aug 2, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants