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

refactor: Remove file I/O from deployment scripts #12015

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

blmalone
Copy link
Contributor

This pull request removes file I/O functionality from deployment scripts, simplifying the codebase and focusing on core deployment logic. Key changes include:

  1. Removed loadInputFile and writeOutputFile methods from various input and output contracts.
  2. Eliminated the run function that accepted file paths as arguments in deployment scripts.
  3. Removed related tests for file I/O operations.
  4. Updated comments to reflect the removal of file-based interactions.

These changes streamline the deployment process, making it more suitable for direct programmatic use in Solidity and Go environments. The update maintains the existing core deployment logic while removing the complexity associated with file-based configurations.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @blmalone and the rest of your teammates on Graphite Graphite

@blmalone blmalone marked this pull request as ready for review September 19, 2024 22:22
@blmalone blmalone requested a review from a team as a code owner September 19, 2024 22:22
@@ -18,15 +18,13 @@ import { BaseDeployIO } from "scripts/utils/BaseDeployIO.sol";
// complement the OP Stack Manager. The script architecture is a bit different than a standard forge
// deployment script.
//
// There are three categories of users that are expected to interact with the scripts:
// 1. End users that want to run live contract deployments.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should keep the end users item, but just indicate they are expected to run the scripts via op-deployer, which also uses the go interface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added here, 23fcc6c feel free to add more details if you have them to this. Otherwise, I can resolve and merge.

Copy link
Contributor

@mds1 mds1 left a comment

Choose a reason for hiding this comment

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

Goodbye file I/O, you will be missed 🫡

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