-
Notifications
You must be signed in to change notification settings - Fork 81
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
Adding NetrWkstaUserEnum implementation #270
Adding NetrWkstaUserEnum implementation #270
Conversation
Thank you very much, @smcintyre-r7, for your valuable suggestions! This was my first experience working with Ruby and the Metasploit Framework, and your insights have been incredibly helpful. |
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 added some suggestions to fix some changes that were missing from the refactoring. I also figured out why the tests are failing and opened a PR with changes in #271 to fix it.
With all of the changes in place, the unit tests should be passing. Would you be able to rebase the branch to squash the history down a bit?
@smcintyre-r7 sure! |
f616103
to
563542e
Compare
Changes are looking good to me. I tested things by updating the Quick patch for testing:
|
3d8abc1
to
eaf2a64
Compare
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.
Unit tests pass from a merged branch incorporating the changes from my last PR and last commit.
This PR adds this request. By this we can modernize an old style Metasploit module here, just like #19170
Verification Steps
bundle install
rake spec
rake spec