build(deps): use mainline go-header #2988
Conversation
|
The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).
|
|
Claude finished @julienrbrt's task in 2m 1s —— View job Code Review CompleteAfter reviewing this PR, I've identified several critical consistency issues and architectural concerns that need to be addressed before merging. Critical Issues1. Inconsistent Go Version Across Monorepo 🔴The most significant issue is the inconsistent Go version update across the repository's modules: Updated to 1.25.0:
Still on older versions:
Root go.mod inconsistency:
This creates a critical monorepo consistency problem:
Fix inconsistent go-header version in root go.mod → 2. Incomplete Dependency Updates 🟡The PR successfully removes the
The pattern suggests the root 3. Core Module Go Version Not Updated 🟡The
This should be updated to Architecture & Quality ReviewCode Quality & Dependency Management ✅Positive aspects:
Go 1.25.0 upgrade implications:
Error Handling & Resilience ✅No error handling changes in this PR - purely dependency management. Security ✅
Performance & Resource Efficiency ✅No performance impact expected - this is a build-time dependency change. Testing & Reliability
|
Summary of ChangesHello @julienrbrt, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the project's dependency configuration by eliminating a custom Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly removes the replace directive for the forked go-header dependency and updates it to a mainline version. This is a good cleanup. However, the dependency updates and the Go version bump to 1.25.0 have only been applied to the submodules in the apps/ directory, while the root go.mod remains unchanged. This inconsistency can lead to unpredictable behavior and build failures depending on the build context. I've left a specific comment on how to resolve this by centralizing the dependency management in the root go.mod file.
| go 1.24.6 | ||
|
|
||
| replace github.com/celestiaorg/go-header => github.com/julienrbrt/go-header v0.0.0-20251008134330-747c8c192fa8 // TODO: to remove after https://github.com/celestiaorg/go-header/pull/347 | ||
| go 1.25.0 |
There was a problem hiding this comment.
The Go version is bumped to 1.25.0 here, but the root go.mod still uses 1.24.6. Similarly, go-header is updated in this submodule but not at the root. To maintain consistency in the monorepo, please update the Go version and dependencies in the root go.mod and then run go mod tidy across the submodules.
| go 1.25.0 | ||
|
|
||
| replace ( | ||
| github.com/evstack/ev-node => ../../. | ||
| github.com/evstack/ev-node/core => ../../core | ||
| ) | ||
|
|
||
| require ( | ||
| github.com/celestiaorg/go-header v0.7.4 | ||
| github.com/celestiaorg/go-header v0.7.5-0.20260116211018-3f61d145c9d2 |
There was a problem hiding this comment.
The Go version and go-header dependency are updated here but not in the root go.mod. This should be made consistent across the repository to avoid build issues. Please update the dependencies and Go version at the root level and ensure all submodules are consistent.
|
Pls hold on until we release it on monday |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2988 +/- ##
=======================================
Coverage 58.98% 58.98%
=======================================
Files 103 103
Lines 9902 9902
=======================================
Hits 5841 5841
Misses 3434 3434
Partials 627 627
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
celestiaorg/go-header#347 got merged. We can finally remove the replace.
I'll keep the repo around as archived not to break previous tags.