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.

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

Add the ImHex.vm package #700

Merged
merged 1 commit into from
Oct 19, 2023
Merged

Add the ImHex.vm package #700

merged 1 commit into from
Oct 19, 2023

Conversation

d35ha
Copy link
Member

@d35ha d35ha commented Oct 18, 2023

No description provided.

@d35ha d35ha requested a review from Ana06 October 18, 2023 11:55
@d35ha d35ha added the 🆕 package New package request/idea/PR label Oct 18, 2023
Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

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

This is a great addition! Thanks for your PR @d35ha 😄 You need to require a exact version before we can merge it, see my comment.

packages/imhex.vm/imhex.vm.nuspec Outdated Show resolved Hide resolved
Install-ChocolateyShortcut -shortcutFilePath $shortcut -targetPath $executablePath
VM-Assert-Path $shortcut

VM-Add-To-Right-Click-Menu $toolName $toolName "`"$executablePath`" `"%1`"" "$executablePath"
Copy link
Member

Choose a reason for hiding this comment

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

[discussion] If we add this tool to the default configuration, we will have two hex editors on the right menu, @mandiant/flare-vm do we want to add this tool to the default config? if so, are we ok with this?

packages/imhex.vm/imhex.vm.nuspec Outdated Show resolved Hide resolved
Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

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

@d35ha the code looks great! can you please squash your commits into just one Add imhex.vm commit in order to maintain a clean commit history? I think this will also fix the CLA issue. I think this is caused by my personal email being co-author of one of the commits (asking around to see if there is a solution for this in general).

Please let me know if you need any help squashing your commits (it can be tricky if you are doing it for the first time). Some documentation that may help: https://www.freecodecamp.org/news/git-squash-commits

@@ -157,7 +157,7 @@ function VM-Check-Reboot {
)
try {
if (Test-PendingReboot){
VM-Write-Log "ERROR" "Host must be rebooted before continuing install of $package.`n"
VM-Write-Log "ERROR" "Host must be rebooted before continuing installation of $package.`n"
Copy link
Member

Choose a reason for hiding this comment

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

Something weird happen while rebasing as I am the author of the commit and you the commiter (you should get authorship) and thisthis change is already in main. But I think merging shouldn't break anything

Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

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

great work, thanks @d35ha!

@Ana06 Ana06 merged commit 61b3c00 into mandiant:main Oct 19, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 package New package request/idea/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants