-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[grid] TomlConfig: migrate TOML library to tomlj/tomlj #14470
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
|
As this TOML library uses a different approach compared to the old one. I would require help from the core maintainers to pass the tests and may have to change the tests and add more accordingly. |
PR Reviewer Guide 🔍
|
PR Code Suggestions ✨
|
|
Updating the TOML parser will lead to a breaking change for users. Example: After the new TOML parser: The new parser requires quotes for the string values. Since our maven Grid package provides access to creating TomlConfig objects, it will be a breaking change for users. We can add a warning with the current code base about the upcoming change, and maybe merge this PR after 2 versions so the end users get a chance to update the TOML. @diemol What do you think? |
|
@pujagani I believe we should go with giving a warning for this as it is a breaking change. As said let's merge and introduce this change after next 2 versions 🚀 |
|
@Delta456 Would like to create a separate PR displaying the warning? |
Sure |
|
Please resolve the conflicts when time permits. Thank you! |
|
@Delta456 Please help resolve the merge conflicts when time permits. Thank you! |
|
@pujagani should be done now. |
User description
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
Fixes #13538 This PR uses a new TOML parser as stated in #13538
Motivation and Context
As this TOML library is more maintained than the one used so lesser chances of problems and vulnerabilities.
Types of changes
Checklist
PR Type
enhancement, dependencies, tests
Description
jtomltotomljin theTomlConfigclass to ensure better maintenance and reduce vulnerabilities.tomljlibrary.tomljlibrary.jtomlwithtomlj.Changes walkthrough 📝
TomlConfig.java
Migrate TOML parsing from `jtoml` to `tomlj`java/src/org/openqa/selenium/grid/config/TomlConfig.java
jtomltotomljfor TOML parsing.tomljAPI.TomlConfigTest.java
Update tests for new TOML library `tomlj`java/test/org/openqa/selenium/grid/config/TomlConfigTest.java
MODULE.bazel
Update Bazel dependencies for TOML library migrationMODULE.bazel
jtomldependency.tomljdependency.maven_install.json
Update Maven artifacts for TOML library migrationjava/maven_install.json
jtomlartifact.tomljartifact.BUILD.bazel
Update Bazel build file for TOML library changejava/src/org/openqa/selenium/grid/config/BUILD.bazel
jtomlwithtomljin dependencies.BUILD.bazel
Update test dependencies for TOML library migrationjava/test/org/openqa/selenium/grid/config/BUILD.bazel
jtomlfrom test dependencies.