-
Notifications
You must be signed in to change notification settings - Fork 142
Comments
Conversation
bugfix for wrong userInfo
@a7ul
a7ul
left a comment
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.
Other than the changes mentioned. it looks good to me.
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.
As mentioned on the issue before . I dont think we should add raiseTestNative Error as part of this module. Could you please remove these.
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.
Also please remove these test functions as well.
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.
ok, I have opened a new PR.
I had thought that only the commits before opening are visible, so I commit my own modify after open it, but it's not the truth. now I make a special branch for this, hope it's ok this time~
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.
by the way , except those test functions, getting and saving the slide is useful for symbol address translation, would you like it to be merged?
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.
Yes. I want to merge everything except the test functions
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.
Please remove only the test functions.
a7ul
commented
Jun 21, 2018
@fatfatson Can you include all the changes in one pr and close the rest ?
bugfix for wrong userInfo