Skip to content

chore: remove Visual Studio-related files for cross-platform cleanup #13702

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

Closed

Conversation

Subham-KRLX
Copy link

This PR removes the build.cmd batch file from the Code Samples/Fib/ folder.

Since this file is specific to Windows and Visual Studio environments, removing it helps streamline the repository for cross-platform development and makes it cleaner for non-Windows contributors.

This is part of a minor cleanup effort to reduce legacy or unused platform-specific files.

@Subham-KRLX Subham-KRLX requested a review from a team as a code owner June 15, 2025 03:00
@github-project-automation github-project-automation bot moved this to Pull Request in cpptools Jun 15, 2025
Copy link
Contributor

@sean-mcmanus sean-mcmanus left a comment

Choose a reason for hiding this comment

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

I don't understand why this file is being removed. It's still referenced in Code Samples/Fib/.vscode/tasks.json for the Windows section of tasks.json.

@Subham-KRLX
Copy link
Author

Thanks for the review, @sean-mcmanus! I've now removed the reference to build.cmd from tasks.json as you pointed out. Let me know if there's anything else you'd like me to update.

"focus": false,
"panel": "shared"
},
"linux": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @Subham-KRLX . Wouldn't this prevent this code sample from working correctly on Windows (i.e. with MinGW G++) ? I think we would want to retain the window specific arguments section.

I believe the build.cmd file is working around a known issue with MinGW that requires its bin directory to be in path when run, in order to properly resolve dependent DLLs. It may be possible to move that work-around into the TypeScript code in order to get rid of build.cmd. But I think we would still want to use of this code sample on Windows.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I think the see the real issue here. This code sample directly uses pthreads, so does not compile on Windows. I think the ideal fix here would be for the sample to be updated to provide a Windows implementation as well.

@Subham-KRLX Subham-KRLX requested a review from Colengms June 19, 2025 02:22
@Subham-KRLX
Copy link
Author

@sean-mcmanus @Colengms: Understood the issues with build.cmd and MinGW. I’ll:

Revert the build.cmd removal (keep Windows support)

Propose a follow-up PR to replace pthreads with C++11 for true cross-platform support

Will that work? Or should we document the Linux/MacOS-only limitation instead?

@Colengms
Copy link
Contributor

@sean-mcmanus @Colengms: Understood the issues with build.cmd and MinGW. I’ll:

Revert the build.cmd removal (keep Windows support)

Propose a follow-up PR to replace pthreads with C++11 for true cross-platform support

Will that work? Or should we document the Linux/MacOS-only limitation instead?

Reverting the build.cmd removal and keeping the Windows support (despite that it can't be built successfully on Windows, due to lack of pthreads) would seem to equate to simply abandoning this PR - no changes needed. Making the sample properly cross-platform seems simple enough. Would you like to open an issue to track that?

@Subham-KRLX
Copy link
Author

Thanks all for the review—looks like the consensus is to keep build.cmd.
I’ll close this PR now.
I’ll also open a new issue to replace the sample’s pthread calls with standard C++11 for cross-platform compatibility.
Reference: #13702.

@Subham-KRLX Subham-KRLX deleted the cleanup/remove-vs-files branch June 27, 2025 05:46
@Subham-KRLX Subham-KRLX restored the cleanup/remove-vs-files branch June 27, 2025 05:46
@Subham-KRLX Subham-KRLX deleted the cleanup/remove-vs-files branch June 27, 2025 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Pull Request
Development

Successfully merging this pull request may close these issues.

3 participants