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.

Sign up for GitHub

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

Jump to bottom

feat: start dotnet feature extraction #958

Merged
merged 25 commits into from
Apr 8, 2022
Merged

feat: start dotnet feature extraction #958

merged 25 commits into from
Apr 8, 2022

Conversation

mike-hunhoff
Copy link
Collaborator

@mike-hunhoff mike-hunhoff commented Apr 5, 2022 •
edited
Loading

closes #951
closes #950
closes #947
closes #946
closes #945
closes #957

  • No CHANGELOG update needed

github-actions[bot]
github-actions bot previously requested changes Apr 5, 2022
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased) section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed

@mike-hunhoff mike-hunhoff marked this pull request as draft April 5, 2022 01:40
@mike-hunhoff mike-hunhoff force-pushed the dotnet-extract branch 2 times, most recently from 794c80f to 25b48c4 Compare April 5, 2022 01:46
@mike-hunhoff mike-hunhoff force-pushed the dotnet-extract branch 2 times, most recently from 042dd8f to ff4f78a Compare April 5, 2022 18:24
@github-actions github-actions bot dismissed their stale review April 6, 2022 21:01

CHANGELOG updated or no update needed, thanks! 😄

@mike-hunhoff mike-hunhoff changed the base branch from master to dotnet-main April 6, 2022 21:15
github-actions[bot]
github-actions bot previously requested changes Apr 6, 2022
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased) section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed

@github-actions github-actions bot dismissed their stale review April 6, 2022 21:26

CHANGELOG updated or no update needed, thanks! 😄

github-actions[bot]
github-actions bot previously requested changes Apr 6, 2022
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased) section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed

@github-actions github-actions bot dismissed their stale review April 6, 2022 21:26

CHANGELOG updated or no update needed, thanks! 😄

@mike-hunhoff mike-hunhoff force-pushed the dotnet-extract branch 4 times, most recently from bc5bef2 to b35b345 Compare April 6, 2022 22:17
@mike-hunhoff mike-hunhoff changed the title feat: dotnet function feature extraction feat: start dotnet feature extraction Apr 6, 2022
@mike-hunhoff mike-hunhoff marked this pull request as ready for review April 6, 2022 22:31
@mike-hunhoff
Copy link
Collaborator Author

updated with PR feedback; pending tests passing ready for final review, squash, and merge into dotnet-main.

williballenthin
williballenthin approved these changes Apr 7, 2022
View reviewed changes
Copy link
Collaborator

@williballenthin williballenthin left a comment

Choose a reason for hiding this comment

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

code looks good, though I'd really like to see tests for these new features. otherwise, I'm not sure i'd call the features complete, yet.

@mr-tz
Copy link
Collaborator

mr-tz commented Apr 8, 2022

yes, let's add tests and then send it!

@mike-hunhoff
Copy link
Collaborator Author

tests added; reopening for review 🚀

williballenthin
williballenthin approved these changes Apr 8, 2022
View reviewed changes
Copy link
Collaborator

@williballenthin williballenthin left a comment

Choose a reason for hiding this comment

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

thank you!

@mike-hunhoff mike-hunhoff merged commit 662be3a into dotnet-main Apr 8, 2022
@mike-hunhoff mike-hunhoff deleted the dotnet-extract branch April 8, 2022 19:30
mr-tz
mr-tz reviewed Apr 9, 2022
View reviewed changes
Copy link
Collaborator

@mr-tz mr-tz left a comment

Choose a reason for hiding this comment

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

LGTM

tests/fixtures.py
elif name.startswith("hello-world"):
return os.path.join(DNFILE_TESTFILES, "hello-world", "hello-world.exe")
elif name.startswith("_1c444"):
return os.path.join(CD, "data", "dotnet", "1c444ebeba24dcba8628b7dfe5fec7c6.exe_")
Copy link
Collaborator

Choose a reason for hiding this comment

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

see DOTNET_DIR

tests/test_dnfile_features.py
@@ -0,0 +1,30 @@
# Copyright (C) 2020 FireEye, Inc. All Rights Reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mandiant 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mr-tz mr-tz mr-tz left review comments

@williballenthin williballenthin williballenthin approved these changes

@github-actions github-actions[bot] github-actions[bot] left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mike-hunhoff @mr-tz @williballenthin