Skip to content
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

Update bazel and its dependencies #14129

Merged
merged 2 commits into from
Jun 13, 2024
Merged

Conversation

shs96c
Copy link
Member

@shs96c shs96c commented Jun 12, 2024

PR Type

Enhancement, Dependencies


Description

  • Updated Bazel version from 7.1.2 to 7.2.0.
  • Updated multiple Bazel dependencies to their latest versions in MODULE.bazel.
  • Added a new protobuf dependency required for closure rules.
  • Removed unused rules_proto_dependencies and rules_proto_toolchains from WORKSPACE.

Changes walkthrough 📝

Relevant files
Enhancement
.bazelversion
Update Bazel version to 7.2.0                                                       

.bazelversion

  • Updated Bazel version from 7.1.2 to 7.2.0.
+1/-1     
WORKSPACE
Remove unused proto dependencies and toolchains                   

WORKSPACE

  • Removed rules_proto_dependencies and rules_proto_toolchains loading.
  • +0/-8     
    Dependencies
    MODULE.bazel
    Update Bazel dependencies and add protobuf                             

    MODULE.bazel

  • Updated multiple Bazel dependencies to newer versions.
  • Added a new dependency for protobuf required for closure rules.
  • +16/-12 

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @qodo-merge-pro qodo-merge-pro bot added dependencies Pull requests that update a dependency file enhancement Review effort [1-5]: 2 labels Jun 12, 2024
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5]

    2

    🧪 Relevant tests

    No

    🔒 Security concerns

    No

    ⚡ Key issues to review

    Dependency Compatibility:
    Ensure that the updated versions of dependencies in MODULE.bazel are compatible with each other and with Bazel 7.2.0.

    Unused Dependency Removal:
    Verify that the removal of rules_proto_dependencies and rules_proto_toolchains from WORKSPACE does not affect any existing functionalities or dependencies.

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Dependency management
    Ensure that removed proto dependencies and toolchains are no longer required by the build process

    Since the rules_proto_dependencies and rules_proto_toolchains were removed, ensure that
    these are no longer required by any part of the build process. If they are still needed,
    consider adding them back or finding an alternative solution.

    WORKSPACE [7-13]

     # This gets us a pre-compiled `protoc`
     
    -load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies", "rules_proto_toolchains")
    +# load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies", "rules_proto_toolchains")
     
    -rules_proto_dependencies()
    +# rules_proto_dependencies()
     
    -rules_proto_toolchains()
    +# rules_proto_toolchains()
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion correctly identifies potential issues with the removal of dependencies that might still be needed, which is crucial for avoiding build failures.

    8
    Compatibility
    Verify compatibility of new dependencies with existing ones by running a full build and test suite

    Ensure that the new dependencies added are compatible with each other and with the
    existing dependencies. This can be done by running a full build and test suite to verify
    that there are no conflicts or issues introduced by the new versions.

    MODULE.bazel [4-9]

    +bazel_dep(name = "aspect_bazel_lib", version = "2.7.7")
    +bazel_dep(name = "aspect_rules_esbuild", version = "0.20.1")
    +bazel_dep(name = "aspect_rules_ts", version = "2.4.2")
    +bazel_dep(name = "bazel_features", version = "1.13.0")
    +bazel_dep(name = "bazel_skylib", version = "1.7.1")
    +...
     
    -
    Suggestion importance[1-10]: 7

    Why: This is a practical suggestion to ensure that newly added dependencies do not introduce conflicts, which is crucial for maintaining a stable build environment.

    7
    Maintainability
    Group related dependencies and add comments to explain their purpose for better readability

    Group related dependencies together and add a comment to explain the purpose of each
    group. This will improve the readability and maintainability of the dependency list.

    MODULE.bazel [4-9]

    +# Aspect dependencies
     bazel_dep(name = "aspect_bazel_lib", version = "2.7.7")
     bazel_dep(name = "aspect_rules_esbuild", version = "0.20.1")
     bazel_dep(name = "aspect_rules_ts", version = "2.4.2")
    +
    +# Bazel features and libraries
     bazel_dep(name = "bazel_features", version = "1.13.0")
     bazel_dep(name = "bazel_skylib", version = "1.7.1")
     ...
     
    Suggestion importance[1-10]: 6

    Why: Grouping and commenting on dependencies enhances maintainability and readability, though it's a moderate improvement rather than a critical fix.

    6
    Best practice
    Pin the versions of all dependencies to avoid issues with future updates

    Consider pinning the versions of all dependencies to avoid potential issues with future
    updates that might introduce breaking changes.

    MODULE.bazel [4-9]

    +bazel_dep(name = "aspect_bazel_lib", version = "2.7.7")
    +bazel_dep(name = "aspect_rules_esbuild", version = "0.20.1")
    +bazel_dep(name = "aspect_rules_ts", version = "2.4.2")
    +bazel_dep(name = "bazel_features", version = "1.13.0")
    +bazel_dep(name = "bazel_skylib", version = "1.7.1")
    +...
     
    -
    Suggestion importance[1-10]: 5

    Why: While pinning versions can help avoid future compatibility issues, the suggestion is somewhat generic and does not address specific risks in the context of the PR changes.

    5

    @shs96c shs96c force-pushed the update-bazel branch 2 times, most recently from f589ade to 54b6701 Compare June 13, 2024 10:04
    @shs96c shs96c merged commit 03bee54 into SeleniumHQ:trunk Jun 13, 2024
    4 checks passed
    @shs96c shs96c deleted the update-bazel branch June 13, 2024 15:04
    sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Oct 29, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    dependencies Pull requests that update a dependency file enhancement Review effort [1-5]: 2
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant