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

for #3919: undo take_mut usage introduced in 11c18b0 #4156

Conversation

koivunej
Copy link
Member

@koivunej koivunej commented May 4, 2023

Commit 11c18b0 introduced take_mut, but, it's actually not necessary here.
take_mut is dangerous because it aborts if the closure handed to it panics.
We have asserts in there, let's not risk an abort.

Alternative to #4153.

koivunej added 2 commits May 4, 2023 21:50
This partially reverts commit 11c18b0
for the take_mut parts.
@koivunej koivunej requested review from a team as code owners May 4, 2023 19:02
@koivunej koivunej requested review from lubennikovaav, shanyp and problame and removed request for a team, lubennikovaav and shanyp May 4, 2023 19:02
@github-actions
Copy link

github-actions bot commented May 4, 2023

Test results for ad70535:


debug build: 233 tests run: 223 passed, 0 failed, 10 skipped (full report)


release build: 233 tests run: 223 passed, 0 failed, 10 skipped (full report)


@problame
Copy link
Contributor

problame commented May 5, 2023

Good changes, bad title. The essence of 11c18b0 was to move the IndexPart construction into the persist_index_part_with_deleted_flag. The introduction of take_mut , reverted in this PR, was munged in by me, my mistake.

I'll rename the PR to what it does, undo take_mut usage introduced in 11c18b0, and merge it into the main PR.

@problame problame changed the title For #3919: revert 11c18b0 for #3919: undo take_mut usage introduced in 11c18b0 May 5, 2023
@problame
Copy link
Contributor

problame commented May 5, 2023

Updated the PR description to the commit message.

@problame problame merged commit 7aba9eb into dkr/deleted-flag-in-remote-index May 5, 2023
@problame problame deleted the joonas/dkr/deleted-flag-in-remote-index branch May 5, 2023 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants