nspcc-dev / neofs-node

NeoFS is a decentralized distributed object storage integrated with the Neo blockchain
https://fs.neo.org
GNU General Public License v3.0
31 stars 38 forks source link

Fix/drop control call #2873

Closed carpawell closed 4 days ago

carpawell commented 2 weeks ago

Overall, should close #2822, hard to say, there were many commits merged about it, and "problem" objects are already dropped.

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 30.00000% with 21 lines in your changes missing coverage. Please review.

Project coverage is 23.61%. Comparing base (450c4e5) to head (932e7b0). Report is 4 commits behind head on master.

Files Patch % Lines
pkg/local_object_storage/metabase/delete.go 27.77% 10 Missing and 3 partials :warning:
pkg/local_object_storage/engine/delete.go 33.33% 7 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2873 +/- ## ========================================== - Coverage 23.69% 23.61% -0.08% ========================================== Files 770 770 Lines 44580 44524 -56 ========================================== - Hits 10562 10514 -48 + Misses 33163 33160 -3 + Partials 855 850 -5 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

carpawell commented 1 week ago

5b6b5a4 explains its changes by 6bf6984, maybe just squash them then?

I prefer a couple of commits. The first one makes Delete work as Inhume, and the second drops no-op code since the previous change. IMO, they are read better this way.

cthulhu-rider commented 1 week ago

the second drops no-op code

i thought that Delete code became less optimal after the 1st. If just no-op, then yeah - separate commits are much better

cthulhu-rider commented 4 days ago

@carpawell worth changelog? the issue occurred in the pub network

carpawell commented 4 days ago

@carpawell worth changelog? the issue occurred in the pub network

Not sure I can find correct words about the whole PR. But if you are about the original issue, we may add it, yes.