Skip to content

options: Forward declare WriteBufferManager - #433

Merged
Yuval-Ariel merged 1 commit into
mainfrom
279-including-instrumented_mutexh-leads-to-compilation-errors
Mar 29, 2023
Merged

options: Forward declare WriteBufferManager#433
Yuval-Ariel merged 1 commit into
mainfrom
279-including-instrumented_mutexh-leads-to-compilation-errors

Conversation

@AmnonHanuhov

@AmnonHanuhov AmnonHanuhov commented Mar 19, 2023

Copy link
Copy Markdown
Contributor

Fixes #279

@AmnonHanuhov AmnonHanuhov added the bug fix Fixes a known bug label Mar 19, 2023
@AmnonHanuhov
AmnonHanuhov requested a review from udi-speedb March 19, 2023 08:39
@AmnonHanuhov AmnonHanuhov self-assigned this Mar 19, 2023
@AmnonHanuhov AmnonHanuhov linked an issue Mar 19, 2023 that may be closed by this pull request
Including instrumented_mutex.h leads to compilation errors #279
Closed
@AmnonHanuhov

AmnonHanuhov commented Mar 19, 2023

Copy link
Copy Markdown
Contributor Author

@Yuval-Ariel

@udi-speedb

udi-speedb commented Mar 20, 2023

Copy link
Copy Markdown
Contributor

@AmnonHanuhov - Please add the issue# to the commit message's title. Also, the commit message is not clear enough IMO.

udi-speedb
udi-speedb previously approved these changes Mar 20, 2023
View reviewed changes
@AmnonHanuhov

AmnonHanuhov commented Mar 21, 2023

Copy link
Copy Markdown
Contributor Author

@AmnonHanuhov - Please add the issue# to the commit message's title. Also, the commit message is not clear enough IMO.

I think the title describes exactly what I did, which is Forward declare WriteBufferManager in the options header. Which info is missing in your opinion?
However, following your comment I added a small explanation to why this is necessary in the description of the commit.

@AmnonHanuhov
AmnonHanuhov force-pushed the 279-including-instrumented_mutexh-leads-to-compilation-errors branch from a2d4320 to b80ce89 Compare March 21, 2023 17:35
@AmnonHanuhov
options: Forward declare WriteBufferManager #279
9b2f95c 
Fixes #279
There is a circular dependency between options.h and write_buffer_manager.h.
In order to fix that, I forward declare the WriteBufferManager class in
options.h and include it in options.cc and in other .cc files which
use WriteBufferManager.
@AmnonHanuhov
AmnonHanuhov force-pushed the 279-including-instrumented_mutexh-leads-to-compilation-errors branch from b80ce89 to 9b2f95c Compare March 23, 2023 08:42
@udi-speedb

udi-speedb commented Mar 26, 2023

Copy link
Copy Markdown
Contributor

@AmnonHanuhov - Please have the issue number in parentheses. If it's not clear, please see other commits. Thanks

@Yuval-Ariel

Yuval-Ariel commented Mar 26, 2023

Copy link
Copy Markdown
Contributor

its fine, i can edit the commit msg when im merging. @udi-speedb , plz approve if there isnt anything else

@udi-speedb

udi-speedb commented Mar 26, 2023

Copy link
Copy Markdown
Contributor

@Yuval-Ariel Nothing else

@udi-speedb
udi-speedb self-requested a review March 28, 2023 08:57
@Yuval-Ariel
Yuval-Ariel merged commit ed310b7 into main Mar 29, 2023
@Yuval-Ariel
Yuval-Ariel deleted the 279-including-instrumented_mutexh-leads-to-compilation-errors branch May 11, 2023 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@udi-speedb udi-speedb udi-speedb approved these changes

Assignees

@AmnonHanuhov AmnonHanuhov

Labels

bug fix Fixes a known bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Including instrumented_mutex.h leads to compilation errors

3 participants

@AmnonHanuhov @udi-speedb @Yuval-Ariel