-
Notifications
You must be signed in to change notification settings - Fork 128
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
Remove napi functions from NexusClasses
#38884
Conversation
5643baf
to
451e8f4
Compare
451e8f4
to
bfd6f25
Compare
*/ | ||
File(NXhandle handle, bool close_handle = false); |
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.
I knew this day would come eventually
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.
Changing the NXhandle
to shared_ptr<::NeXus::File>
is a great way to make sure things get cleaned up with less effort. I also like that you removed some (not used by the outside) variables from load()
and cleaned up the method. I didn't realize how many different copy constructors would be needed for shared_ptr
.
Remove napi functions from `NexusClasses`
Description of work
Summary of work
This finishes un-reverting work in PR #38791 .
This will remove all instances of
napi
functions fromNexusClasses
, which also finishes the process of consolidating access of NeXus throughNeXus::File
.The
NXDataSetTyped::load
method was gently refactored to remove thek
,l
parameters, which are never used, as well as the branching logic for those being non-negative, which never happened.Changes to
NeXus::File
are minimal, including adding additional constructors, and an assignment operator.Related to #38332
Further detail of work
To test:
Windows build from commit bfd6f25 (NOT tip of branch)

Windows build NOT re-run on commit de12700, but this should not impact any behavior.
Reviewer
Please comment on the points listed below (full description).
Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review. If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed.
Code Review
Functional Tests
Does everything look good? Mark the review as Approve. A member of
@mantidproject/gatekeepers
will take care of it.Gatekeeper
If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.