Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Coalesced Buffer Communication #1192
base: develop
Are you sure you want to change the base?
Coalesced Buffer Communication #1192
Changes from 110 commits
539ba5f
d1c1274
e42ee36
40a0a02
00ce27b
64d655e
c3ddf52
74f9c33
ee04547
cc14c89
295e8a3
d8fbd65
566f36d
8a9a1bc
75667ab
d56bc78
a35fccf
6436fdc
5fd8de5
95db032
9c4010f
b4efb3d
995913e
160c77f
6355185
12df3b6
b55659c
1be047d
1b405dc
7f5b944
b0dd208
d8ae6e8
d0d0194
6bafb0a
07f62e2
809dcb1
43c376b
505426d
e60b82c
4d49ce5
ee58f4d
59c8680
dc54426
b823b74
5649581
17f25e0
cff847b
8cc982c
72ec3ac
0ff61c0
f5f7bba
7c320cb
20e9765
977b2a3
fe8a2af
2e35467
7c80de1
ec10ab2
675895e
9bb4747
cef604f
655dc39
d786b86
ece20a3
4116362
25c2c51
d7ba65d
91771ad
5cddc0d
207c2dc
ac01d92
fa53f72
24aa55f
f72ee8f
a194eba
3b21bbc
8670107
f2d8c0c
deeccf4
c22cf96
d5d4328
255e85a
3941117
9159c93
52778c5
451b246
43c4efa
5ff9c60
3adb853
06231e1
b14432f
24a9733
1e5e4d4
1f1c5f3
0c3131e
08d9c13
a694074
3353b52
408ecb3
06bd5d3
0bd053e
0183590
975ce4a
867af0a
b573337
5648a82
1bcfdba
7edde1e
5fefb71
9ae28c8
e95f691
c67a5fb
6aa7dcd
0f3795d
22ecb2e
822a859
8739373
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we think this works for all downstreams including kharma, artemis and riot I am in favor of the default being true. If there's some doubt, we should maybe change the default to false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I tend to lean toward default
false
until there is more downstream testing. To make sure it is passing regression tests, it needs to be set totrue
for now though (or we would have to change all the parameter input). There is some discussion of this above though where @brryan suggested we keeptrue
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with it being default
true
. But I would also be fine modifying all the tests to set it to true manually.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm in principle also happy with default true (assuming that all downstream codes work/perform as expected as others already noted).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the hacks? Might be worth saying what to do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I removed
CoalescedBuffer::Compare
at some point, so this note isn't so useful (and the hacks are a bit hard to quickly describe here). As a result, I just removed this point from the doc.