Skip to content

Conversation

tac0turtle
Copy link
Contributor

Overview

This pr removes defaults in favor of being explicit for users

Copy link
Contributor

github-actions bot commented Sep 5, 2025

The latest Buf updates on your PR. Results from workflow CI and Release / buf-check (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed⏩ skipped✅ passed✅ passedSep 9, 2025, 2:33 PM

@@ -84,7 +87,7 @@ func InitCmd() *cobra.Command {

// Add flags to the command
rollconf.AddFlags(initCmd)
initCmd.Flags().String(rollgenesis.ChainIDFlag, "evolve-test", "chain ID")
initCmd.Flags().String(rollgenesis.ChainIDFlag, "", "chain ID must be set. It is used in the genesis file and to identify the network (examples: ev-1. xo-1)")
Copy link
Member

Choose a reason for hiding this comment

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

We should make the flag as required or put chain-id as first command argument

}

if tt.dataNamespace != "" {
assert.Equal(t, tt.dataNamespace, dataNS)
} else if tt.namespace != "" {
assert.Equal(t, tt.namespace, dataNS)
} else {
assert.Equal(t, "rollkit-headers", dataNS) // Falls back to default namespace
assert.Equal(t, "", dataNS) // Falls back to default namespace
Copy link
Member

Choose a reason for hiding this comment

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

Nit rename comment

Copy link
Member

Choose a reason for hiding this comment

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

No need to add flags here. We can set it in default config above once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for some we need the flag as the primary issue is that parseconfig makes a new config and does not use the one provided to the command. the one provided to the command. The test are weird and follow an odd flow but the command is needed.

Copy link

codecov bot commented Sep 8, 2025

Codecov Report

❌ Patch coverage is 5.55556% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.02%. Comparing base (0584153) to head (38a70e1).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/cmd/run_node.go 0.00% 13 Missing ⚠️
pkg/config/config.go 20.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2629      +/-   ##
==========================================
- Coverage   69.17%   69.02%   -0.16%     
==========================================
  Files          74       74              
  Lines        7803     7816      +13     
==========================================
- Hits         5398     5395       -3     
- Misses       1955     1970      +15     
- Partials      450      451       +1     
Flag Coverage Δ
combined 69.02% <5.55%> (-0.16%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants