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 a url keyword or argument to Agent#initialize #19

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

CharlieWWW94
Copy link
Contributor

Adds the url: kwarg to the Ronin::Web::Browers::Agent initializer, as outlined in issue #16. Upon initialization the browser will navigate to this url.

Hope this contribution is useful - did my best to conform to code conventions across the rest of the repo. Please do let me know of any changes that are needed. Eager to contribute in any way that might be helpful 👍

@CharlieWWW94 CharlieWWW94 changed the title Add url kwarg to Ronin::Web::Browser::Agent initializer Add a url keyword or argument to Agent#initialize Jul 23, 2024
Copy link
Member

@postmodern postmodern left a comment

Choose a reason for hiding this comment

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

Minor stylistic changes.

I would also copy/paste the @param tag into lib/ronin/web/browser.rb for the .new method.

Also, if it's not too much work, could you rebase against the 0.2.0 branch? I like to merge new features into the next upcoming minor version branch. Otherwise I can manually merge your branch into 0.2.0.

lib/ronin/web/browser/agent.rb Outdated Show resolved Hide resolved
lib/ronin/web/browser/agent.rb Outdated Show resolved Hide resolved
@postmodern postmodern added the feature New Feature label Jul 23, 2024
@postmodern postmodern added this to the 0.2.0 milestone Jul 23, 2024
@CharlieWWW94 CharlieWWW94 changed the base branch from main to 0.2.0 July 24, 2024 07:23
@CharlieWWW94
Copy link
Contributor Author

Minor stylistic changes.

I would also copy/paste the @param tag into lib/ronin/web/browser.rb for the .new method.

Also, if it's not too much work, could you rebase against the 0.2.0 branch? I like to merge new features into the next upcoming minor version branch. Otherwise I can manually merge your branch into 0.2.0.

Thanks for the feedback! Have completed those, rebased against 0.2.0 and updated the base branch of the PR to 0.2.0.

Copy link
Member

@postmodern postmodern left a comment

Choose a reason for hiding this comment

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

Minor changes. Looks good except for those two things.

lib/ronin/web/browser/agent.rb Outdated Show resolved Hide resolved
lib/ronin/web/browser.rb Outdated Show resolved Hide resolved
Copy link
Member

@postmodern postmodern left a comment

Choose a reason for hiding this comment

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

Last request for change I swear! Did some testing to figure out what values go_to actually accepts. Turns out URI::HTTP and anything that Addressable::URI can accept are valid.

lib/ronin/web/browser/agent.rb Outdated Show resolved Hide resolved
@postmodern postmodern merged commit fef6232 into ronin-rb:0.2.0 Jul 24, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants