Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

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

Closed

Conversation

@abhishek-malani-zz
Copy link

@abhishek-malani-zz abhishek-malani-zz commented Mar 16, 2020 •
edited
Loading

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

  • Change to the site (I am attaching a screenshot showing the before and after)
  • Code example added (and I also added the example to all translated languages)
  • Improved translation
  • Added new translation (and I also added a notice to each document missing translation)

Checklist

  • I have read the contributing document.
  • I have used hugo to render the site/docs locally and I am sure it works.

Copy link

CLAassistant commented Mar 16, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

Copy link
Author

abhishek-malani-zz commented Mar 16, 2020 via email

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> .

Copy link
Member

harsha509 commented Mar 16, 2020 •
edited
Loading

Sure will make changes and create a new pr
...
On Mon, Mar 16, 2020, 1:12 PM Sri Harsha @.***> wrote: @harsha509 requested changes

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,
Harsha.

Copy link
Author

abhishek-malani-zz commented Mar 16, 2020 via email

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> .

Copy link
Member

Ahh okay.. is there anything else I can help you with? I am fairly new and could use your guidance.. Regards, Abhishek

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,
Harsha

Copy link
Member

diemol commented Mar 17, 2020

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.
harsha509 and abhishek-malani-zz reacted with thumbs up emoji

Copy link
Member

We're moving away from desired capabilities, please use chromeOptions to set the capability

setting an option should be preferred.
@abhishek-malani can you make that change?

Copy link
Member

Hi @abhishek-malani ,

Selenium-4 alpha 5 for python bindings is out now. Can you please update the PR with suggested changes.

Thanks,
Harsha.

Copy link
Author

Sure..

Copy link
Member

Hi @abhishek-malani ,

Can you please add code changes in other translated pages and sign the CLA !

Thanks,
Harsha

Copy link
Author

abhishek-malani-zz commented Mar 22, 2020 via email

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> .

harsha509 added a commit that referenced this pull request Mar 22, 2020
Copy link
Member

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,
Harsha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@harsha509 harsha509 harsha509 requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /