-
Notifications
You must be signed in to change notification settings - Fork 38
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
rm/Drop placement code copy #2478
rm/Drop placement code copy #2478
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2478 +/- ##
==========================================
+ Coverage 29.26% 29.29% +0.03%
==========================================
Files 399 399
Lines 30390 30346 -44
==========================================
- Hits 8893 8891 -2
+ Misses 20761 20719 -42
Partials 736 736
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Waits for #2470. |
rdy |
ee1ce55
to
f33a730
Compare
CHANGELOG! |
You mean changed placement? Copy-paste/removal of the SDK's code is not needed to be changelogged, IMO. |
Yeah, I'm not sure it affects users in any way, but it's a change and formally even a protocol-level one (old nodes compute these things differently). |
f33a730
to
34ee6fb
Compare
It was added only due to a temporary SDK's interface problem. SDK RC10 has solved it. NOTE: generally, the placement for container estimations changed with this commit: another pivot is used. We do this change cause SDK used to have a bug related to the placement func and placement changes are expected after RC9 -> RC10 update anyway. Signed-off-by: Pavel Karpy <[email protected]>
34ee6fb
to
59a8b5a
Compare
It was added only due to a temporary SDK's interface problem. SDK RC10 has solved it.
NOTE: generally, the placement for container estimations changed with this commit: another pivot is used. We do this change cause SDK used to have a bug related to the placement func and placement changes are expected after RC9 -> RC10 update anyway.