Skip to content

Conversation

@aguspe
Copy link
Contributor

@aguspe aguspe commented May 31, 2025

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

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

    Labels

    B-build Includes scripting, bazel and CI integrations C-rb Ruby Bindings Review effort 2/5

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants