Skip to content

Revert "[rb] Add support for beta chrome" #15837

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 31, 2025
Merged

Revert "[rb] Add support for beta chrome" #15837

merged 1 commit into from
May 31, 2025

Conversation

aguspe
Copy link
Contributor

@aguspe aguspe commented May 31, 2025 •
edited by qodo-merge-pro bot
Loading

User description

Reverts #15417

PR Type

enhancement

Description

  • Remove support for Chrome Beta in Bazel build and Ruby specs

  • Refactor browser pinning script to only handle stable Chrome

  • Clean up Bazel configuration and test targets for Chrome Beta

  • Simplify browser data selection and repository setup

Changes walkthrough 📝

Relevant files
Configuration changes
3 files
browsers.bzl
Remove Chrome Beta data selection logic                                   
+0/-22   
repositories.bzl
Remove Chrome Beta repository and archive definitions       
+2/-78   
MODULE.bazel
Remove Chrome Beta repositories from Bazel module usage   
+0/-4     
Tests
3 files
tests.bzl
Remove Chrome Beta from Ruby browser test matrix                 
+0/-25   
BUILD.bazel
Remove Chrome Beta from integration test browser list       
+0/-1     
BUILD.bazel
Remove Chrome Beta from Chrome integration test browsers 
+1/-6     
Enhancement
1 files
pinned_browsers.py
Refactor to pin only stable Chrome, remove Beta logic       
+56/-42 
Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    •

    @selenium-ci selenium-ci added C-rb Ruby Bindings B-build Includes scripting, bazel and CI integrations labels May 31, 2025
    @qodo-merge-pro Qodo Merge Pro
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Function Signature

    The function signature for get_chrome_milestone() doesn't match its implementation. It appears to be retrieving Chrome channel information but doesn't return a milestone value directly.

     
    def get_chrome_milestone():
    parser = argparse.ArgumentParser()
    parser.add_argument(
        "--chrome_channel", default="Stable", help="Set the Chrome channel"
    )
    args = parser.parse_args()
    channel = args.chrome_channel
    Missing Data Dependency

    The data dependency for common/extensions was removed but might still be needed for Chrome tests to function properly.

     
        browsers = ["chrome"],  # No need to run in other browsers.
)

    @qodo-merge-pro Qodo Merge Pro
    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Rename function for clarity

    The get_chrome_milestone() function name is misleading as it actually returns
    the full version information, not just the milestone. Rename the function to
    better reflect its purpose, such as get_chrome_version_info().

    scripts/pinned_browsers.py [29-35]

    -def get_chrome_milestone():
+def get_chrome_version_info():
     parser = argparse.ArgumentParser()
     parser.add_argument(
         "--chrome_channel", default="Stable", help="Set the Chrome channel"
     )
     args = parser.parse_args()
     channel = args.chrome_channel
    • Apply / Chat
    Suggestion importance[1-10]: 5

    __

    Why: The function name get_chrome_milestone() is indeed misleading since it returns full version information rather than just the milestone number. This is a valid readability improvement that would make the code more self-documenting.

    		Low
    Improve variable naming

    The variable name chrome_milestone is misleading since the function returns the
    full version information object, not just the milestone number. Rename it to
    chrome_version_info to better reflect its content.

    scripts/pinned_browsers.py [525-527]

    -chrome_milestone = get_chrome_milestone()
-content = content + chrome(chrome_milestone)
-content = content + chromedriver(chrome_milestone)
+chrome_version_info = get_chrome_milestone()
+content = content + chrome(chrome_version_info)
+content = content + chromedriver(chrome_version_info)
    • Apply / Chat
    Suggestion importance[1-10]: 4

    __

    Why: The variable name chrome_milestone is misleading as it stores the complete version information object. Renaming to chrome_version_info would improve code clarity and is consistent with the function naming suggestion.

    		Low
    • More

    @aguspe aguspe merged commit f52bb20 into trunk May 31, 2025
    26 checks passed
    @aguspe aguspe deleted the revert-15417-rb_add_support_for_chrome_beta branch May 31, 2025 17:57
    @aguspe aguspe mentioned this pull request Jun 7, 2025
    Merged
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers
    No reviews
    Assignees
    No one assigned
    Labels
    B-build Includes scripting, bazel and CI integrations C-rb Ruby Bindings Review effort 2/5
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.
    2 participants
    @aguspe @selenium-ci