-
Notifications
You must be signed in to change notification settings - Fork 564
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
dotnet update: emit enclosing class information for nested classes #1913
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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.
Great work @bkojusner - your code is coming along nicely. Please see my comments and suggestions. Let me know here if you have any questions 🚀
Co-authored-by: Mike Hunhoff <mike.hunhoff@gmail.com>
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.
Good work @bkojusner this is coming together nicely. Please address my comments and re-request a review from me when you're ready. Let me know if you have any questions along the way 😃.
Co-authored-by: Mike Hunhoff <mike.hunhoff@gmail.com>
Co-authored-by: Mike Hunhoff <mike.hunhoff@gmail.com>
Co-authored-by: Mike Hunhoff <mike.hunhoff@gmail.com>
Co-authored-by: Mike Hunhoff <mike.hunhoff@gmail.com>
Co-authored-by: Mike Hunhoff <mike.hunhoff@gmail.com>
Co-authored-by: Mike Hunhoff <mike.hunhoff@gmail.com>
Uploaded two test samples for the unit tests at mandiant/capa-testfiles#224. |
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.
@bkojusner nice work! A few small changes for you to address and then we are ready to merge 🚀
nested_class_table = {} | ||
|
||
# Used to find nested classes in typedef | ||
for _rid, nestedclass in iter_dotnet_table(pe, dnfile.mdtable.NestedClass.number): |
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.
Is _rid
being used here? If not, see my suggestion above
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.
great job @bkojusner LGTM 🚀
woohoo congrats @bkojusner! |
awesome! 🥳 |
Draft PR with progress to resolve #1780