-
Notifications
You must be signed in to change notification settings - Fork 129
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 mass ws deletion bug #18914
Fix mass ws deletion bug #18914
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with example scripts and played with tying to pass invalid input (e.g. no workspaces) to algorithm. All seems well. See comment below from code review but this is basically ready to go I think.
} | ||
alg->setProperty("WorkspaceList", vecWsNames); | ||
executeAlgorithmAsync(alg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should put all this in a private method? void MantidUI::deleteWorkspaces()
or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it already is!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it is!
@NickDraper Also checked this with Giovanni's Mantid project and I cannot reproduce. |
LGTM |
add DeleteWorkspaces algorithm, use it in GUI re #17729 (cherry picked from commit 9953f7c) clang format and release notes re #17729 (cherry picked from commit b6ba0e5) Resolve cppcheck warning re #17729 (cherry picked from commit 8827cbd) Resolve small syntax error (cherry picked from commit 1891682) Add release note for mass workspace fix
Fixes #17729
Added a DeleteWorkspaces algorithm, use it in GUI when deleting multiple workspaces, all it does is iterate over the list deleting workspaces.
To test:
* Test that deleting many workspaces does not crash*
* Test that deleting few very large data sets does not disturb the user experience (too much)*
RELEASE NOTES
docs/source/release/framework.rst
Reviewer
Please comment on the following (full description):
Code Review
Functional Tests
Do changes function as described? Add comments below that describe the tests performed?
How do the changes handle unexpected situations, e.g. bad input?
Has the relevant documentation been added/updated?
Is user-facing documentation written in a user-friendly manner?
Has developer documentation been updated if required?
Does everything look good? Comment with the ship it emoji but don't merge. A member of
@mantidproject/gatekeepers
will take care of it.