-
Notifications
You must be signed in to change notification settings - Fork 0
Mega-refactor #15
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
Merged
Merged
Mega-refactor #15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The module now has a shared run_nargo_command helper that consolidates argument building, logging, and dry-run handling. This avoids duplicating command execution logic across command modules.
The new tests verify the behavior of --dry-run, --verbose and --pkg flags across various commands including check, clean, rebuild, cairo gen and evm gen.
The changes introduce a new Backend trait that provides a common interface for Cairo and EVM proof system implementations, along with a factory pattern for creating backend instances. Command dispatching is updated to use this new abstraction.
The commit adds downcasting support via `as_any_mut()` to the Backend trait and changes all Backend trait methods to take `&mut self`. This enables safe downcasting to concrete backend types and allows backends to maintain internal state.
The changes add automatic contract declaration capability to the Cairo deploy command, while deprecating the standalone 'declare' command in favor of this integrated approach.
Adds command execution abstraction with Runner trait and implements auto-declare behavior for Cairo deploy. Includes comprehensive test coverage and documentation for the new functionality. Deprecates the separate Cairo declare command in favor of integrated auto-declare.
The changes simplify backend configuration and improve the deployment flow by: - Replacing individual backend setters with a unified configure() method - Moving declare functionality to be internal-only - Updating user-facing messages to promote auto-declare
The commit verifies deployment output differences between auto-declare and no-declare modes through integration tests, ensuring the feature works as expected.
The commit migrates all bb command executions to use the centralized command runner abstraction, providing consistent handling of dry-run mode and logging across the codebase. The main changes: - Add run_bb_command helper in common.rs for standardized bb execution - Update all bb operations to take Config and use the runner - Remove redundant dry-run message printing - Add TODO noting bb backend can be removed in next checkpoint
The commit moves command execution to a runner abstraction providing consistent dry-run handling, logging, and testability. Legacy direct execution functions have been removed from bb, foundry, garaga, and nargo backend modules.
The added configuration parameter allows for dry-run mode and runner abstraction in Garaga functions, consistent with other commands.
The changes migrate various command executions (nargo, garaga, foundry) to use a common runner abstraction, removing direct backend calls in favor of a more unified approach to command execution.
The change removes a redundant forge command execution function in favor of a more general tool runner interface.
This commit removes the direct garaga command execution backend in favor of the unified runner-based approach. All garaga functionality is now handled through the common command runner interface for better testing and consistency.
The changes consolidate all external tool execution into two main functions: run_tool() for standard execution and run_tool_capture() for capturing stdout. This replaces the previous tool-specific helper functions which are now deprecated.
The commit adds command history tracking to DryRunRunner and unifies external command execution through a new run_tool() interface. This makes it easier to test command execution flows by inspecting the recorded command history rather than parsing stdout.
This extends the dry run runner to generate tool-specific fake outputs for testing and store captured outputs in command history.
This commit restructures utility code into more focused modules: - Move common I/O operations into new `io.rs` module - Create dedicated `format.rs` module for text formatting - Rename `output.rs` to `log.rs` for logging utilities - Add documentation and examples for each module - Add unit tests for new functionality The changes improve code organization and make the utilities more maintainable without changing their behavior.
Adds improved error handling for file operations and introduces a dedicated error module for the Cairo backend with structured error types.
The commit adds more descriptive error context using color_eyre's WrapErr trait throughout the codebase. This helps with debugging by providing better error messages that include file paths and operation details when failures occur.
test structure with fixture-based workflow testing - Use golden file snapshots to verify build artifacts and output - Ensure thread-safety with ScopedDir guards for parallel tests - Implement DryRunRunner for command execution verification - Add comprehensive test coverage documentation
80573f2 to
68ced07
Compare
68ced07 to
8b47c42
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
bargo v0.2.0 — Mega-refactor PR (Phases 1 → 7)
Highlights
handle_*helpers; global flags now reach every command.run_nargo_commandhelper + smoke tests; build is-Dwarningsclean.Backend+ polymorphic dispatch; stateful back-ends; Cairo deploy auto-declares.CmdSpec+Runner;DryRunRunnerwith command history & fake outputs.util/intoio,log,format,timer; addedcmd_spec!macro.eyre::WrapErron every external boundary;CairoError/EvmErrorenums.What’s new
CairoBackend,EvmBackend) replace match ladders.--auto-declare/--no-declare.Runner; history plus stubbed output.Developer impact
Error chains now show context, for example:
Verification
cargo check --workspace --all-targets --all-features -Dwarnings→ cleancargo test→ 92 tests greencargo doc --workspace --no-deps -q→ no warningsunsafecode addedReview tips
src/runner.rs,src/backend.rs, everything inutil/.phase‑4,phase‑6, …) to ease review.tests/goldens/; refresh them withUPDATE_GOLDENS=1 cargo test.