BUGFIX: Asset usage prevent orphaned asset usage entries in case of a rebase with conflicts #5406
+39
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dlubitz
reviewed
Dec 19, 2024
Comment on lines
+77
to
+81
| if ($eventInstance instanceof WorkspaceWasRebased && $eventInstance->hadConflicts) { | ||
| // because we don't know which changes were discarded in a conflict, we discard all changes and will build up the index on succeeding calls (with the kept reapplied events) | ||
| $this->discardWorkspace($eventInstance->getWorkspaceName()); | ||
| return; | ||
| } |
There was a problem hiding this comment.
This could also go into the match below.
What happens to the remaining events if the workspace was rebased sucessfully? Aren't they re-applied too? So do we really need to distinguish here?
There was a problem hiding this comment.
Ok, question was answered in weekly.
But still this can go into the match below.
Comment on lines
+62
to
+66
| public function getWorkspaceName(): WorkspaceName | ||
| { | ||
| return $this->sourceWorkspaceName; | ||
| } | ||
|
|
There was a problem hiding this comment.
Ohhh,... hmm. This seems to be dangerous. In your case "workspaceName" is "sourceWorkspaceName" but in other cases its "targetWorkspaceName". That's why we didn't implelemt the interface here earlier.
Also, is that needed at any point?
mhsdesign
commented
Dec 20, 2024
| /** | ||
| * Indicates if failing changes were discarded during a forced rebase {@see RebaseErrorHandlingStrategy::STRATEGY_FORCE} or if all events in the workspace were kept. | ||
| */ | ||
| public bool $hadConflicts |
There was a problem hiding this comment.
as discussed in the daily we want to rename this to hasSkippedEvents
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Upgrade instructions
Review instructions
Checklist
FEATURE|TASK|BUGFIX!!!and have upgrade-instructions