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

fix db_notify.cpp not included in disable unity build db set #589

Merged
merged 1 commit into from
Jan 23, 2018

Conversation

ihla
Copy link

@ihla ihla commented Jan 20, 2018

this PR fixes linker error when GRAPHENE_DISABLE_UNITY_BUILD=true

@abitmore abitmore added the build About build process label Jan 20, 2018
Copy link
Member

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

thank you for fixing this. i confirmed the project will not build when GRAPHENE_DISABLE_UNITY_BUILD is true and adding db_notify.cpp alone is not enough; it needs the includes in this pull request as well to compile.

i am not really sure why someone will disable the feature but thats another thing(please excuse my ignorance in the subject).

when enabled(default) project compiles and run fine too so the changes will not affect the default build and fix the problem when flag is used.

thanks again, approving now and will be merged soon.

@ihla
Copy link
Author

ihla commented Jan 23, 2018

there is only one reason why I have enabled it - when working with the IDE like Clion one can use many of the convenient IDE features like code navigation and browsing for which the files must be included in the project - the Clion takes only files included in the build from the cmake file, I don't observer any compilation slowing down with GRAPHENE_DISABLE_UNITY_BUILD when running on my notebook with SSD

@oxarbitrage oxarbitrage merged commit 197ceca into bitshares:develop Jan 23, 2018
@oxarbitrage
Copy link
Member

interesting, i use clion, i am going to give a try to that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build About build process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants