-
-
Notifications
You must be signed in to change notification settings - Fork 8
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 a Ronin::Recon::Config#to_yaml
method
#161
Conversation
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.
Overall looks good, just minor suggestions.
I may have realized a problem with my Config::Workers#initialize
implementation. Since it converts the input value to a Array
, and we dump that Array
value back out to the YAML file, this means the workers:
will always end up being an explicit Array of worker IDs. This means that when we release a new version of ronin-recon
, which adds new default built-in workers, users won't have those workers enabled by default because their config file will contain an explicit Array of worker IDs. I think I need to refactor Workers#initialize
to preserve the original value so we load a Hash of worker_id: true|false
and save the workers:
back to a Hash. I'll need to think about this.
032be97
to
9e8f021
Compare
00aa5aa
to
fc0b678
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.
Noticed one last thing. Looks good though.
I will need to re-think how to properly store the list of workers as a Hash by default.
fc0b678
to
6e02eec
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.
Missing @since
tags on new methods.
6e02eec
to
aecf45e
Compare
Closing #159
I don't know how (and if it's even possible) to save workers with indents :/