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

cmake: Ignore build subdirectories within the source directory #302

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

hebasto
Copy link
Owner

@hebasto hebasto commented Aug 5, 2024

It was planned to clean up the .gitignore file during the removal of the Autotools and MSVC legacy build systems. However, reviewers requested it earlier.

Therefore, delivering it now :)

/*build*
!/build-aux
!/build_msvc

Copy link

Choose a reason for hiding this comment

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

the mentioned cmake-build-debug and cmake-build-debug-coverage folders were created outside of build by default:

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm not a CLion expert, but it seems inappropriate to create build artifacts anywhere outside of the CMake's designated binary tree. Is this configurable?

Copy link

@l0rinc l0rinc Aug 5, 2024

Choose a reason for hiding this comment

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

yes, but it's not the default, I'll open an issue

Copy link

Choose a reason for hiding this comment

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

Copy link
Owner Author

Choose a reason for hiding this comment

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

I can see that "cmake-build-debug" is a configured name of the build directory for the "Debug" build type. It is OK to have multiple build directories simultaneously. The suggested pattern /*build* covers all such cases.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

Thanks, will let them decide, they have all the context here

.gitignore Show resolved Hide resolved
Copy link

@l0rinc l0rinc left a comment

Choose a reason for hiding this comment

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

ACK ff39f0b

@l0rinc
Copy link

l0rinc commented Aug 5, 2024

ACK 57cdcdf

Copy link

@TheCharlatan TheCharlatan left a comment

Choose a reason for hiding this comment

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

lgtm

@hebasto hebasto merged commit d847137 into cmake-staging Aug 6, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants