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

Fix: recursive delete calls no longer use activePublish #587

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

smlpt
Copy link
Contributor

@smlpt smlpt commented Apr 23, 2024

This should fix the slow deletion process for large groups of objects. Previously every child node deletion and every recursive call used activePublish = true, which slowed down sciview by a lot.

@smlpt smlpt added the bugfix This PR contains a bugfix label Apr 23, 2024
@smlpt smlpt requested a review from kephale April 23, 2024 08:27
Copy link
Member

@ctrueden ctrueden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. My only question is whether the event that does get published, about the group deletion itself, has a way to know which nodes were deleted recursively. Presumably not. But maybe we don't care.

@smlpt
Copy link
Contributor Author

smlpt commented Apr 23, 2024

Good question, but I'd also assume no.

And I tested this with the Protein you can add via the Add menu. The deletion is almost instant now.

@skalarproduktraum
Copy link
Member

I'd agree that if it's known that the parent node is gone is enough 👍

@skalarproduktraum
Copy link
Member

Thanks, @smlpt 👍

@skalarproduktraum skalarproduktraum merged commit 7943804 into main Apr 23, 2024
4 checks passed
@skalarproduktraum skalarproduktraum deleted the fix/publish-deletions branch April 23, 2024 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR contains a bugfix prague-hackathon-2024
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants