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: merge genesisapplier & genesisdeployment #63

Merged
merged 2 commits into from
Jul 26, 2024
Merged

chore: merge genesisapplier & genesisdeployment #63

merged 2 commits into from
Jul 26, 2024

Conversation

hamdiallam
Copy link
Contributor

No description provided.

@hamdiallam hamdiallam requested a review from a team as a code owner July 26, 2024 15:16
Copy link
Contributor

coderabbitai bot commented Jul 26, 2024

Walkthrough

The recent changes involve a significant refactoring of package imports and structures within the codebase. The genesisdeployment package has been renamed to genesis, leading to updates in how related entities are accessed throughout the code. Additionally, the genesis/applier.go file has been streamlined by removing the Config struct, simplifying function signatures, and improving the clarity of the code. Overall, these modifications enhance maintainability without altering the underlying functionality.

Changes

Files Change Summary
config/chain.go Renamed import from genesisdeployment to genesis. Updated references to GeneratedGenesisDeployment accordingly. Adjusted ChainID, GenesisJSON, and L1ChainID fields for SourceChain, OPChainA, and OPChainB.
genesis/applier.go Changed package name from genesisapplier to genesis. Removed Config struct. Renamed and simplified UpdateGenesisWithConfig to ApplyChainID with a clearer function signature.
genesis/deployment.go Changed package name from genesisdeployment to genesis. No changes in logic, only in naming.

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

@hamdiallam hamdiallam changed the title merge genesisapplier & genesisdeployment chore: merge genesisapplier & genesisdeployment Jul 26, 2024
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: 1

Outside diff range, codebase verification and nitpick comments (1)
genesis/applier.go (1)

Line range hint 25-26:
Improve error message specificity.

The error message "error marshaling genesis" could be more specific by including the original error.

-    return nil, fmt.Errorf("error marshaling genesis")
+    return nil, fmt.Errorf("error marshaling genesis: %v", err)

Comment on lines 13 to 14

err := json.Unmarshal(genesisJson, &genesis)
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve error message specificity.

The error message "unable to parse genesis json" could be more specific by including the original error.

-    return nil, fmt.Errorf("unable to parse genesis json")
+    return nil, fmt.Errorf("unable to parse genesis json: %v", err)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
err := json.Unmarshal(genesisJson, &genesis)
err := json.Unmarshal(genesisJson, &genesis)
if err != nil {
return nil, fmt.Errorf("unable to parse genesis json: %v", err)
}

@hamdiallam hamdiallam merged commit f0ef708 into main Jul 26, 2024
3 checks passed
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