-
Notifications
You must be signed in to change notification settings - Fork 21
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
Allow custom options; Bump version (v1.0.0) #33
Conversation
\cc @miherlosev |
\cc @kirovboris |
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.
/r-
} | ||
|
||
async startBrowser (browser, url, jobOptions = {}, timeout = null) { | ||
jobOptions = { ...jobOptions, ...browser }; |
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.
[minor/recommendation] This method huge and not readable, it'd be nice to simplify it.
@AndreyBelym we are interested in these changes too, where are we at with the rework? |
This change will allow me to use saucelabs-connector on our production build server, on which we have a shared sauce connect tunnel. Looks like it's been approved and has no conflicts. Can it be integrated? |
It's only partially approved, I still need to fix a lot of things. |
@AndreyBelym Sorry to bug, but do you intend to address this anytime soon? |
Yep, I will try to finish it after finishing other tasks |
@AndreyBelym - do you still plan to address this issue? |
Yes, I still have plans to finish it, but currently there is no progress. |
…new message: "Sauce Labs API request failed", fix the readme example
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.
LGTM, all thanks to @Farfurix 👍
No description provided.