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

build(deps): fix x/protocolpool version in x/feegrant #22840

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

robert-zaremba
Copy link
Collaborator

@robert-zaremba robert-zaremba commented Dec 12, 2024

Description

Similar to #22839, but targetting main


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title, you can find examples of the prefixes below:
  • confirmed ! in the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • updated the relevant documentation or specification, including comments for documenting Go code
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

  • New Features

    • Updated Go version to 1.23.3.
    • Added support for the cosmossdk.io/x/protocolpool module as a new dependency.
  • Bug Fixes

    • Updated replacement directives for various cosmossdk.io modules.

@robert-zaremba robert-zaremba requested a review from a team as a code owner December 12, 2024 00:53
Copy link
Contributor

coderabbitai bot commented Dec 12, 2024

📝 Walkthrough

Walkthrough

The changes in this pull request primarily involve updates to the x/feegrant/go.mod file. The Go version has been upgraded to 1.23.3, and a new dependency on the cosmossdk.io/x/protocolpool module has been introduced. Additionally, replacement directives for several cosmossdk.io modules have been modified, including a new replacement for the added cosmossdk.io/x/protocolpool.

Changes

File Change Summary
x/feegrant/go.mod Updated Go version to 1.23.3, added cosmossdk.io/x/protocolpool as a dependency, and modified replacement directives for various cosmossdk.io modules.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant GoModule
    participant ProtocolPool

    User->>GoModule: Update Go version to 1.23.3
    User->>GoModule: Add cosmossdk.io/x/protocolpool
    GoModule->>ProtocolPool: Include ProtocolPool dependency
    GoModule->>GoModule: Update replacement directives
Loading

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>, please review it.
    • Generate unit testing code 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 testing code 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Experiment)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@tac0turtle
Copy link
Member

what error is this giving you that is causing issues? tests and everything runs fine in the sdk. not sure what the issue is

@robert-zaremba
Copy link
Collaborator Author

robert-zaremba commented Dec 12, 2024

in v0.52:

go: cosmossdk.io/x/[email protected]: invalid version: unknown revision a1bc045b3190

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.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
x/feegrant/go.mod (1)

Line range hint 3-3: Invalid Go version specified

The Go version 1.23.3 is invalid. Go versions follow semantic versioning with only two components (major.minor), and the latest stable version is 1.22.x.

Please update to a valid Go version:

-go 1.23.3
+go 1.22
📜 Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between f266328 and d9559b1.

⛔ Files ignored due to path filters (1)
  • x/feegrant/go.sum is excluded by !**/*.sum
📒 Files selected for processing (1)
  • x/feegrant/go.mod (1 hunks)
🔇 Additional comments (2)
x/feegrant/go.mod (2)

Line range hint 52-52: Verify protocolpool version compatibility

The added protocolpool dependency version 0.0.0-20230925135524-a1bc045b3190 should be verified for compatibility with other module versions, especially with core and store versions.

✅ Verification successful

Let me gather more information about the protocolpool dependency versions.


Based on the analysis of the codebase, I can now provide a final response regarding the protocolpool version compatibility.

Protocolpool version is compatible with core dependencies

The protocolpool module is tightly integrated with the codebase and is being used as a local module (via replace directives) rather than an external dependency. All the go.mod files show consistent replace directives pointing to the local protocolpool module, ensuring version compatibility across the codebase.

Key observations:

  • The module is referenced via replace directives in multiple modules (staking, slashing, gov, etc.)
  • It's properly integrated with core modules like distribution which has been updated to delegate community pool functionality to protocolpool
  • The module follows the same versioning pattern as other core modules in the codebase
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the protocolpool version is compatible with other dependencies
gh api graphql -f query='
{
  repository(owner: "cosmos", name: "cosmos-sdk") {
    object(expression: "main") {
      ... on Commit {
        history(path: "x/protocolpool/go.mod") {
          nodes {
            committedDate
            oid
            messageHeadline
          }
        }
      }
    }
  }
}'

Length of output: 17751


Script:

#!/bin/bash
# Check protocolpool dependency in go.mod files
fd -e mod -x cat {} \; | grep -A 5 -B 5 "protocolpool"

# Check if protocolpool is used in the codebase and its imports
rg -A 3 "protocolpool" --type go

Length of output: 103727


190-190: Verify the protocolpool module path

The replace directive for protocolpool points to a relative path ../protocolpool. Please ensure this path is correct and the module exists at this location.

✅ Verification successful

Based on the search results, I can see that the protocolpool module exists within the repository at x/protocolpool/. The replace directive in the review comment points to ../protocolpool, which is correct as it references the module relative to the feegrant module's location at x/feegrant/.

The replace directive path is correct

The protocolpool module is properly located within the repository at x/protocolpool/ and the relative path ../protocolpool from x/feegrant/go.mod correctly points to it. This is consistent with the repository's module structure and the ongoing modularization effort.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the existence of the protocolpool module
if [ -d "../protocolpool" ]; then
  echo "protocolpool module directory exists"
  if [ -f "../protocolpool/go.mod" ]; then
    echo "protocolpool go.mod exists"
    cat "../protocolpool/go.mod"
  fi
fi

Length of output: 28


Script:

#!/bin/bash
# Let's try to find the protocolpool module within the repository
fd protocolpool

# Check if there's a go.mod file mentioning protocolpool
rg -l "protocolpool" --type go-mod

# Look for any references to protocolpool in the codebase
rg "protocolpool" -g "!*.sum" -g "!*.mod"

Length of output: 119519

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.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)
x/feegrant/go.mod (2)

Line range hint 3-3: Invalid Go version specified

The Go version 1.23.3 is invalid. Go versions follow semantic versioning with only two components (e.g., 1.22), and the latest stable version is 1.22.1 as of March 2024.

Apply this diff to fix the Go version:

-go 1.23.3
+go 1.22

189-191: LGTM: Replacement directive aligns with module structure

The addition of the protocolpool replacement directive follows the same pattern as other module replacements. Note the TODO comment about removing these replacements after spinning out all modules.

Consider creating a tracking issue for the TODO about removing replacements after spinning out all modules.

📜 Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between f266328 and d9559b1.

⛔ Files ignored due to path filters (1)
  • x/feegrant/go.sum is excluded by !**/*.sum
📒 Files selected for processing (1)
  • x/feegrant/go.mod (1 hunks)
🔇 Additional comments (1)
x/feegrant/go.mod (1)

Line range hint 52-52: Verify the protocolpool version

The PR aims to fix the version of x/protocolpool. Please confirm if the pseudo-version v0.0.0-20230925135524-a1bc045b3190 from September 2023 is the intended version.

✅ Verification successful

The protocolpool version is correctly managed by the replace directive

The pseudo-version v0.0.0-20230925135524-a1bc045b3190 is not a concern because the module is actually replaced with a local version through the replace directive: cosmossdk.io/x/protocolpool => ../protocolpool. This is the standard approach for managing internal module dependencies within a monorepo, where modules are developed together and their versions are controlled by the main repository.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the available versions and commit history of protocolpool

# Check if the commit exists in the repository
gh api repos/cosmos/cosmos-sdk/commits/a1bc045b3190 --jq '.sha'

# List tags containing protocolpool changes
git tag --sort=-creatordate | while read -r tag; do
    if git ls-tree -r "$tag" | grep -q "x/protocolpool"; then
        echo "$tag"
    fi
done

Length of output: 233


Script:

#!/bin/bash
# Let's try a different approach to verify the protocolpool version
# First, check the current version in go.mod
grep "cosmossdk.io/x/protocolpool" x/feegrant/go.mod

# Then check for any references to protocolpool in the codebase
rg "protocolpool" --type go

# Also check the imports in Go files
rg "cosmossdk.io/x/protocolpool" --type go

Length of output: 73821


Script:

#!/bin/bash
# Let's check the replace directive and the actual module location
grep -A 1 "cosmossdk.io/x/protocolpool" x/feegrant/go.mod

# Check if there's a go.mod in the protocolpool directory
cat ../protocolpool/go.mod 2>/dev/null || echo "No go.mod found in protocolpool directory"

# Check if there are any version tags in the protocolpool directory
git -C ../protocolpool tag -l 2>/dev/null || echo "Not a git repository or no tags found"

Length of output: 532

@julienrbrt julienrbrt changed the title deps: fix x/protocolpool version in x/feegrant build(deps): fix x/protocolpool version in x/feegrant Dec 12, 2024
@tac0turtle tac0turtle added this pull request to the merge queue Dec 12, 2024
Merged via the queue into main with commit 02fe18c Dec 12, 2024
83 of 89 checks passed
@tac0turtle tac0turtle deleted the robert/fix-protocolpool-ver-main branch December 12, 2024 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants