Skip to content

[java] Removes deprecated events classes #11819#12578

Closed
dev-velo wants to merge 5 commits intoSeleniumHQ:trunkfrom
dev-velo:feat/11819
Closed

[java] Removes deprecated events classes #11819#12578
dev-velo wants to merge 5 commits intoSeleniumHQ:trunkfrom
dev-velo:feat/11819

Conversation

@dev-velo
Copy link
Contributor

Thanks for contributing to Selenium!
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

Remove deprecated event classes as described in #11819

Motivation and Context

Classes were deprecated in 4.0

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

removes deprecated code

@titusfortner
Copy link
Member

Because this is both a major change and is something there isn't a lot of conversation around, we would like to have documentation for the new code and some kind of blog post talking about how to upgrade, etc. That's why we've kept this code around long after it should have been removed. Would you be interested in helping us with that?

@diemol
Copy link
Member

diemol commented Aug 23, 2023

@RevealOscar do you think you can help us document EventFiringDecorator and WebDriverListener?

@dev-velo
Copy link
Contributor Author

dev-velo commented Sep 2, 2023

@titusfortner @diemol
wrote a mini guide talking about the transition, and how to port over code with examples. I was going to commit it as a md file but figured it would be easier to edit on google docs since it may need some extra work. Made a copy of the original so feel free to modify this file. About to delve into adding javadocs to the code.

*/
default void afterPerform(WebDriver driver, Collection<Sequence> actions) {}

default void beforeResetInputState(WebDriver driver) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, why were these removed from the interface?

"afterGet",
"afterAnyWebDriverCall get",
"afterAnyCall get"));
"beforeAnyWebDriverCall getCurrentUrl",
Copy link
Contributor

Choose a reason for hiding this comment

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

I appreciate adding exhaustive tests. But it might be a good idea to split the PRs out. For example, removal could be one PR and one PR could be adding tests. It will help with cleaner commits.

@pujagani pujagani added the C-java Java Bindings label Sep 28, 2023
@pujagani
Copy link
Contributor

pujagani commented Sep 28, 2023

@RevealOscar Thank you for the blog post. It's great. Would you help us add it as a blog post/tutorial as part of the Selenium website? https://github.com/SeleniumHQ/seleniumhq.github.io.

@pujagani
Copy link
Contributor

pujagani commented Oct 9, 2023

@RevealOscar Can you please help resolve the conflicts?

@dev-velo
Copy link
Contributor Author

@pujagani yea I can resolve the conflicts and split the pr into 2. in the process of getting a new machine so havent been able to look at the pr

@pujagani
Copy link
Contributor

Thank you @RevealOscar! Let us know if we can help you in anyway.

@dev-velo
Copy link
Contributor Author

dev-velo commented Nov 24, 2023

@pujagani apologies for the delay but I finally was able to get a new laptop and picked the work back up.

feel free to close this PR as I split this work into separate PR's.
Mini blog pr -> SeleniumHQ/seleniumhq.github.io#1533
Add exhaustive tests -> #13198
Add javadoc -> #13199
Remove deprecated classes -> #13200

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

Labels

C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants