-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
page loading strategy for python #325
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
Conversation
docs_source_files/content/webdriver/page_loading_strategy.en.md
Outdated
Show resolved
Hide resolved
Sure will make changes and create a new pr
…On Mon, Mar 16, 2020, 1:12 PM Sri Harsha ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
In docs_source_files/content/webdriver/page_loading_strategy.en.md
<#325 (comment)>
:
> @@ -244,7 +244,14 @@ public class pageLoadStrategy {
}
{{< / code-panel >}}
{{< code-panel language="python" >}}
-# Please raise a PR
+from selenium import webdriver
+from selenium.webdriver.common.desired_capabilities import DesiredCapabilities
Hi @abhishek-malani <https://github.com/abhishek-malani> ,
Thanks for the PR. Apologies, but I'v couple of things to discuss here
1. Code sample didn't work for me when I tested with selenium alpha
bindings.
2. We're moving away from desired capabilities, please use
chromeOptions to set the capability
3. Can you please make code changes to other translated pages too.
Regards,
Harsha
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#325 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADU35ZR7ZPTCTAYOA7U5XR3RHXJ6PANCNFSM4LL34PYA>
.
|
And most importantly, the reason i have left it blank because this feature is not available in python bindings yet. This feature has landed with commit SeleniumHQ/selenium@f878211 and we still need to wait for alpha version to get rolled out. Regards, |
Ahh okay.. is there anything else I can help you with? I am fairly new and
could use your guidance..
Regards,
Abhishek
…On Mon, Mar 16, 2020, 1:23 PM Sri Harsha ***@***.***> wrote:
Sure will make changes and create a new pr
… <#m_5889292222447351356_>
On Mon, Mar 16, 2020, 1:12 PM Sri Harsha *@*.***> wrote: ***@***.***
<https://github.com/Harsha509>* requested changes on this pull request.
------------------------------ In docs_source_files/content/webdriver/
page_loading_strategy.en.md <#325 (comment)
<#325 (comment)>>
: > @@ -244,7 +244,14 @@ public class pageLoadStrategy { } {{< / code-panel
>}} {{< code-panel language="python" >}} -# Please raise a PR +from
selenium import webdriver +from
selenium.webdriver.common.desired_capabilities import DesiredCapabilities
Hi @abhishek-malani <https://github.com/abhishek-malani>
https://github.com/abhishek-malani , Thanks for the PR. Apologies, but
I'v couple of things to discuss here 1. Code sample didn't work for me when
I tested with selenium alpha bindings. 2. We're moving away from desired
capabilities, please use chromeOptions to set the capability 3. Can you
please make code changes to other translated pages too. Regards, Harsha —
You are receiving this because you were mentioned. Reply to this email
directly, view it on GitHub <#325 (review)
<#325 (review)>>,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ADU35ZR7ZPTCTAYOA7U5XR3RHXJ6PANCNFSM4LL34PYA
.
And most importantly, the reason i have left it blank becuase this feature
is not available in python bindings yet. This feature has landed with
commit ***@***.***
<SeleniumHQ/selenium@f878211>
and we still need to wait for alpha version to get rolled out.
Regards,
Harsha.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#325 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADU35ZTARMKMTQJJKX6V4NLRHXLF5ANCNFSM4LL34PYA>
.
|
For this PR. I suggest you to wait until the next alpha is released. Meanwhile, you can contribute for any section in this document. Regards, |
Just FYI, an alpha will be released this week, then it is fine to have this, so we could merge this as soon as it uses options instead of desired capabilities. |
setting an option should be preferred. |
Hi @abhishek-malani , Selenium-4 alpha 5 for python bindings is out now. Can you please update the PR with suggested changes. Thanks, |
Sure.. |
Made the requested changes
Hi @abhishek-malani , Can you please add code changes in other translated pages and sign the CLA ! Thanks, |
Sure
…On Sun, Mar 22, 2020 at 6:47 PM Sri Harsha ***@***.***> wrote:
Hi @abhishek-malani <https://github.com/abhishek-malani> ,
Can you please add code changes in other translated pages and sign the CLA
!
Thanks,
Harsha
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#325 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADU35ZVFU7KUFSTVHRX24TTRIYFVHANCNFSM4LL34PYA>
.
|
Hi @abhishek-malani , Thanks for the PR. I've merged one of the PR which contains your changes. Usually we do create a single PR for any type changes for all translated pages. Providing multiple is fine, but it triggers the 2 builds for each PR to get merged which consumes lot of time. So referring to all your PRs done for translated pages i've made a single commit. Hope you're fine with it. Regards, |
Thanks for contributing to the Selenium site and documentation!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
Motivation and Context
Types of changes
Checklist