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

chore(unification): rename KeeperBuilder to Builder #2471

Merged
merged 2 commits into from
Mar 25, 2024

Conversation

robert-zaremba
Copy link
Member

@robert-zaremba robert-zaremba commented Mar 24, 2024

Description

Small update to unify the name used in modules

Summary by CodeRabbit

  • Refactor
    • Simplified naming convention for builder types and methods across various modules, enhancing readability and consistency.
    • Updated function signatures and struct types to align with the new naming convention, ensuring uniformity across the codebase.
  • Tests
    • Adjusted test suites and unit tests to reflect changes in builder naming and initialization, maintaining test reliability and coherence with the updated code structure.

@robert-zaremba robert-zaremba added the skip-e2e-test Skip the e2e tests label Mar 24, 2024
@robert-zaremba robert-zaremba requested a review from a team as a code owner March 24, 2024 12:20
Copy link
Contributor

coderabbitai bot commented Mar 24, 2024

Walkthrough

The overall change across the codebase involves a significant renaming effort, transitioning from the use of KeeperBuilder to Builder in various contexts. This renaming affects type definitions, function names, and method calls, streamlining the naming convention and potentially clarifying the purpose and functionality of the builders within the system. The modifications span multiple files and modules, indicating a concerted effort to enhance code readability and maintainability.

Changes

Files Change Summary
app/app.go
x/.../keeper/intest/keeper_test.go
x/.../keeper/intest/msg_server_test.go
x/.../keeper/keeper.go
x/.../keeper/unit_test.go
x/ugov/keeper/intest/keeper.go
x/ugov/keeper/keeper.go
x/uibc/module/module.go
x/uibc/quota/genesis.go
x/uibc/quota/grpc_query.go
x/uibc/quota/intest/suite_test.go
x/uibc/quota/keeper.go
x/uibc/quota/msg_server.go
x/uibc/quota/unit_test.go
x/uibc/uics20/ibc_module.go
x/uibc/uics20/ibc_module_test.go
x/uibc/uics20/ics4_wrapper.go
x/uibc/uics20/ics4_wrapper_test.go
Renamed KeeperBuilder to Builder and updated corresponding function calls and type definitions across multiple files and modules.

Poem

🐇 "In the realm of code, a change took flight,
From KeeperBuilder to Builder, into the night.
Across the land, the files did align,
With names so clear, the purpose did shine.
So hop we now, with joy and cheer,
For cleaner code, the path is clear!" 🌟

  • CodeRabbit 🎩✨

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link

codecov bot commented Mar 24, 2024

Codecov Report

Attention: Patch coverage is 44.44444% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 69.38%. Comparing base (7f05ad4) to head (a089eb3).
Report is 418 commits behind head on main.

❗ Current head a089eb3 differs from pull request most recent head 5046ae4. Consider uploading reports for the commit 5046ae4 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2471      +/-   ##
==========================================
- Coverage   75.38%   69.38%   -6.01%     
==========================================
  Files         100      185      +85     
  Lines        8025    10909    +2884     
==========================================
+ Hits         6050     7569    +1519     
- Misses       1589     2712    +1123     
- Partials      386      628     +242     
Files Coverage Δ
x/metoken/keeper/keeper.go 100.00% <ø> (ø)
x/ugov/keeper/keeper.go 80.00% <ø> (ø)
x/uibc/quota/keeper.go 100.00% <100.00%> (ø)
x/uibc/uics20/ibc_module.go 53.01% <ø> (ø)
x/uibc/uics20/ics4_wrapper.go 100.00% <100.00%> (ø)
x/uibc/quota/msg_server.go 0.00% <0.00%> (ø)
x/uibc/quota/genesis.go 0.00% <0.00%> (ø)
x/uibc/quota/grpc_query.go 0.00% <0.00%> (ø)

... and 168 files with indirect coverage changes

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 6dccda9 and a089eb3.
Files selected for processing (18)
  • app/app.go (4 hunks)
  • x/metoken/keeper/intest/keeper_test.go (1 hunks)
  • x/metoken/keeper/intest/msg_server_test.go (4 hunks)
  • x/metoken/keeper/keeper.go (1 hunks)
  • x/metoken/keeper/unit_test.go (1 hunks)
  • x/ugov/keeper/intest/keeper.go (1 hunks)
  • x/ugov/keeper/keeper.go (1 hunks)
  • x/uibc/module/module.go (1 hunks)
  • x/uibc/quota/genesis.go (2 hunks)
  • x/uibc/quota/grpc_query.go (1 hunks)
  • x/uibc/quota/intest/suite_test.go (1 hunks)
  • x/uibc/quota/keeper.go (2 hunks)
  • x/uibc/quota/msg_server.go (1 hunks)
  • x/uibc/quota/unit_test.go (1 hunks)
  • x/uibc/uics20/ibc_module.go (1 hunks)
  • x/uibc/uics20/ibc_module_test.go (1 hunks)
  • x/uibc/uics20/ics4_wrapper.go (1 hunks)
  • x/uibc/uics20/ics4_wrapper_test.go (1 hunks)
Additional comments: 28
x/ugov/keeper/intest/keeper.go (1)
  • 23-23: The renaming from NewKeeperBuilder to NewBuilder aligns with the PR's objective to unify naming conventions. Ensure that the NewBuilder function is correctly implemented in the keeper package and that its usage here is consistent with its definition.
x/ugov/keeper/keeper.go (1)
  • 18-18: The introduction of the NewBuilder function and the Builder type aligns with the PR's objective to unify naming conventions. The implementation is correct and facilitates the construction of a Keeper instance with the necessary dependencies.
x/uibc/uics20/ics4_wrapper.go (2)
  • 17-17: The change in the ICS4 struct from quota.KeeperBuilder to quota.Builder aligns with the PR's objective to unify naming conventions. Ensure that the quota.Builder type is correctly implemented and that its usage here is consistent with its definition.
  • 20-20: The update to the NewICS4 function signature to use quota.Builder is correctly implemented and aligns with the PR's objective. Ensure that the quota.Builder type is correctly defined.
x/metoken/keeper/unit_test.go (1)
  • 25-25: The renaming from NewKeeperBuilder to NewBuilder in the initSimpleKeeper function aligns with the PR's objective to unify naming conventions. Ensure that the NewBuilder function is correctly implemented and that its usage here is consistent with its definition.
x/uibc/quota/unit_test.go (1)
  • 28-28: The renaming from NewKeeperBuilder to NewBuilder in the initKeeper function aligns with the PR's objective to unify naming conventions. Ensure that the NewBuilder function is correctly implemented and that its usage here is consistent with its definition.
x/uibc/quota/keeper.go (2)
  • 16-17: The introduction of the Builder type and the NewBuilder function aligns with the PR's objective to unify naming conventions. The implementation is correct and facilitates the construction of a Keeper instance with the necessary dependencies.
  • 25-29: The NewBuilder function is correctly implemented and aligns with the PR's objective. Ensure that the Builder type is correctly defined and that its usage here is consistent with its definition.
x/uibc/quota/msg_server.go (2)
  • 14-14: Renaming KeeperBuilder to Builder in the msgServer struct aligns with the PR's objective to standardize naming conventions across the codebase.
  • 18-18: The update in the NewMsgServerImpl function to accept Builder instead of KeeperBuilder is consistent with the renaming effort. However, it's noted that this line was not covered by tests.
x/uibc/quota/grpc_query.go (2)
  • 14-14: Renaming KeeperBuilder to Builder in the Querier struct aligns with the PR's objective to standardize naming conventions across the codebase.
  • 17-18: The update in the NewQuerier function to accept Builder instead of KeeperBuilder is consistent with the renaming effort. However, it's noted that these lines were not covered by tests.
Verification successful

The verification process has revealed that while there may not be direct tests specifically named to test the NewQuerier function, the function is indeed used across various parts of the codebase, including in test setups. This indicates that the NewQuerier function is likely covered indirectly by tests that utilize these setups, such as in suite_test.go and grpc_query_test.go files. Therefore, the concern about the lack of test coverage for the NewQuerier function following the renaming effort is addressed through its integration and usage in the system's overall testing framework.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify if NewQuerier function is covered by tests after renaming
rg --type go "NewQuerier" --files-with-matches | xargs rg "func Test.*NewQuerier"

Length of output: 81



Script:

#!/bin/bash
# Search for any test files that reference the NewQuerier function to check for indirect testing
rg --type go "NewQuerier" --files-with-matches | xargs rg --type go "func Test"

# Additionally, verify the usage of the NewQuerier function in the codebase to understand its testing context better
rg --type go "NewQuerier"

Length of output: 2435

x/uibc/quota/intest/suite_test.go (1)
  • 110-110: The change from NewKeeperBuilder to NewBuilder in the initKeeper function call is consistent with the PR's objective. This update is part of the test suite, ensuring that the renaming is also reflected in the tests.
x/uibc/uics20/ics4_wrapper_test.go (1)
  • 55-55: The change from NewKeeperBuilder to NewBuilder in the TestSendPacket function call is consistent with the PR's objective. This update is part of the test suite, ensuring that the renaming is also reflected in the tests.
x/uibc/module/module.go (2)
  • 86-86: Renaming KeeperBuilder to Builder in the AppModule struct aligns with the PR's objective to standardize naming conventions across the codebase.
  • 89-89: The update in the NewAppModule function to accept Builder instead of KeeperBuilder is consistent with the renaming effort.
x/uibc/uics20/ibc_module_test.go (1)
  • 65-65: The change from NewKeeperBuilder to NewBuilder in the TestIBCOnRecvPacket function call is consistent with the PR's objective. This update is part of the test suite, ensuring that the renaming is also reflected in the tests.
x/metoken/keeper/intest/keeper_test.go (1)
  • 57-57: The change from NewKeeperBuilder to NewBuilder in the initTestSuite function call is consistent with the PR's objective. This update is part of the test suite, ensuring that the renaming is also reflected in the tests.
x/uibc/uics20/ibc_module.go (2)
  • 27-27: Renaming KeeperBuilder to Builder in the ICS20Module struct aligns with the PR's objective to standardize naming conventions across the codebase.
  • 34-34: The update in the NewICS20Module function to accept Builder instead of KeeperBuilder is consistent with the renaming effort.
app/app.go (4)
  • 284-284: The renaming of UIbcQuotaKeeperB from uibcquota.KeeperBuilder to uibcquota.Builder aligns with the PR's objective to standardize naming conventions across the codebase. This change enhances readability and consistency.
  • 464-464: The use of ugovkeeper.NewBuilder to instantiate UGovKeeperB follows the PR's renaming convention and is correctly applied here. This change contributes to the overall goal of improving codebase consistency.
  • 497-497: Similarly, the instantiation of MetokenKeeperB using metokenkeeper.NewBuilder adheres to the PR's renaming strategy. This consistent naming helps clarify the role of these components within the application.
  • 541-541: The renaming of UIbcQuotaKeeperB to use uibcquota.NewBuilder in this context is consistent with the PR's objectives. It's good to see the renaming effort being applied thoroughly across the codebase.
x/metoken/keeper/intest/msg_server_test.go (4)
  • 610-610: The renaming from KeeperBuilder to Builder is consistent with the PR's objective and is correctly applied here.

Ensure that the renaming does not inadvertently affect the functionality of the tests. It might be beneficial to verify that all tests still pass and behave as expected after the renaming.

  • 707-707: The renaming from KeeperBuilder to Builder is correctly applied here as well.

Similar to the previous comment, please ensure that the renaming does not affect the functionality of the tests.

  • 1351-1351: Renaming applied correctly in the context of depegging tests.

As with the previous instances, verify that the functionality of the tests remains intact after the renaming.

  • 1448-1448: The renaming from KeeperBuilder to Builder is applied correctly here as well.

Please verify the functionality of the tests after the renaming.

x/uibc/quota/genesis.go Show resolved Hide resolved
x/uibc/quota/genesis.go Show resolved Hide resolved
Copy link
Collaborator

@gsk967 gsk967 left a comment

Choose a reason for hiding this comment

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

LGTM

@robert-zaremba robert-zaremba disabled auto-merge March 25, 2024 15:06
@robert-zaremba robert-zaremba added this pull request to the merge queue Mar 25, 2024
Merged via the queue into main with commit 764a5c4 Mar 25, 2024
20 of 21 checks passed
@robert-zaremba robert-zaremba deleted the robert/unify-builder-name branch March 25, 2024 16:00
@robert-zaremba robert-zaremba removed the skip-e2e-test Skip the e2e tests label Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants