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

[Cherry-pick to 1.7] Removed TerminalVelocity usages from IDL #5110

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

godlytalias
Copy link
Contributor

Cherry-pick of #5107

@godlytalias godlytalias requested a review from codendone February 6, 2025 18:08
@godlytalias
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@godlytalias godlytalias changed the title Removed TerminalVelocity usages from IDL [Cherry-pick] Removed TerminalVelocity usages from IDL Feb 6, 2025
@godlytalias godlytalias enabled auto-merge (squash) February 6, 2025 18:28
@codendone codendone changed the title [Cherry-pick] Removed TerminalVelocity usages from IDL [Cherry-pick to 1.7] Removed TerminalVelocity usages from IDL Feb 6, 2025
@DrusTheAxe
Copy link
Member

If TerminalVelocityFeatures-BackgroundTask.h is obsolete do you plan to delete dev\common\TerminalVelocityFeatures-BackgroundTask.xml and .h ?

@godlytalias godlytalias merged commit ccfdcca into release/1.7-stable Feb 6, 2025
26 checks passed
@godlytalias godlytalias deleted the user/godlyalias/alwaysenabled_1.7 branch February 6, 2025 20:26
@godlytalias
Copy link
Contributor Author

If TerminalVelocityFeatures-BackgroundTask.h is obsolete do you plan to delete dev\common\TerminalVelocityFeatures-BackgroundTask.xml and .h ?

@DrusTheAxe As of now, the usage is still there in implementation files, and yeah it need to be cleaned up. Should it be just before the stable release? Or is there any guideline?

@DrusTheAxe
Copy link
Member

@DrusTheAxe As of now, the usage is still there in implementation files, and yeah it need to be cleaned up. Should it be just before the stable release? Or is there any guideline?

Ideally 'dead code' is cleaned up when it becomes 'dead'. As the say, the best time to act is yesterday. The 2nd best time to act is today :-)

You can make the change at any time in main. Whether it's worthy to bring to the vNext release branch before stable (.0) release is a question for @codendone, shiproom, office hours, etc.

If inert it's not worthy to bring to release/x.y-stable servicing updates. If it's harmful then it's a bug worthy of servicing (though in this case it's hard to see how it would a problematic worthy of servicing).

@codendone would know about documented guidelines

@codendone
Copy link
Contributor

I'm not aware of any written guidance currently about cleanup.

+1 to the comments from @DrusTheAxe on risky/benefit. Since this is just cleanup which should not cause any issues for 1.7, I suggest doing the cleanup only in main--it isn't worth the cherry-pick and additional churn in the release branch.

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.

3 participants