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

avoid copying Python files from source to build space #3

Merged
merged 1 commit into from
Feb 16, 2016

Conversation

dirk-thomas
Copy link
Member

Based on the discussion in #2 (comment)

@dirk-thomas dirk-thomas added the in review Waiting for review (Kanban column) label Feb 16, 2016
@dirk-thomas dirk-thomas self-assigned this Feb 16, 2016
@esteve
Copy link
Member

esteve commented Feb 16, 2016

I'm far from a CMake expert, but why is copying from the source to build space a bad practice?

@esteve
Copy link
Member

esteve commented Feb 16, 2016

Oops, I just saw your comment on #2 (comment)

@wjwwood
Copy link
Member

wjwwood commented Feb 16, 2016

I agree that having the installed code come directly from the source space is better.

For the code in the build space used for running tests I think it is less important. The tests rely on both generated and compiled code so building before iterating on the test is already pretty common. We could have continued to use the file copy for the build space setup, and installed from the source space for a similar effect without the need for the complicated and somewhat confusing init file that mocks the source space.

But since you've already implemented it we should probably keep it. +1

tfoote added a commit that referenced this pull request Feb 16, 2016
…ild_space

avoid copying Python files from source to build space
@tfoote tfoote merged commit 01cb2b9 into initial Feb 16, 2016
@tfoote tfoote deleted the avoid_copying_files_from_source_to_build_space branch February 16, 2016 21:33
@tfoote tfoote removed the in review Waiting for review (Kanban column) label Feb 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants