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

feat: refactor configuration #41

Merged
merged 3 commits into from
Feb 14, 2025
Merged

feat: refactor configuration #41

merged 3 commits into from
Feb 14, 2025

Conversation

atanmarko
Copy link
Contributor

@atanmarko atanmarko commented Feb 12, 2025

Description

Refactor dependencies between configurations.

aggchain-proof-service is now separate service.

Resolves #43

PR Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added or updated tests that comprehensively prove my change is effective or that my feature works

@atanmarko atanmarko self-assigned this Feb 12, 2025
@atanmarko atanmarko marked this pull request as ready for review February 12, 2025 14:08
@atanmarko atanmarko requested a review from a team as a code owner February 12, 2025 14:08
@atanmarko atanmarko requested review from 0xaatif, Ekleog-Polygon and Freyskeyd and removed request for a team February 12, 2025 14:08
@atanmarko
Copy link
Contributor Author

Resolves #43

Ekleog-Polygon
Ekleog-Polygon previously approved these changes Feb 13, 2025
Copy link

@Ekleog-Polygon Ekleog-Polygon left a comment

Choose a reason for hiding this comment

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

LGTM! Though you'll likely need to wrangle with git, considering this looks like it's built on an older version of #29

Base automatically changed from feat/aggchain-proof-builder to main February 13, 2025 10:41
@atanmarko atanmarko dismissed Ekleog-Polygon’s stale review February 13, 2025 10:41

The base branch was changed.

@atanmarko
Copy link
Contributor Author

Though you'll likely need to wrangle with git, considering this looks like it's built on an older version of #29

There is nothing one cherry-pick and force push can not resolve 😜

@atanmarko atanmarko force-pushed the feat/refactor-configuration branch from 4492d64 to e918f8f Compare February 13, 2025 11:03
Ekleog-Polygon
Ekleog-Polygon previously approved these changes Feb 13, 2025
Copy link

@Ekleog-Polygon Ekleog-Polygon left a comment

Choose a reason for hiding this comment

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

Showing with 205 additions and 1,896 deletions.

Considering the PR size, guess I'll review the PR again and not the "compare changes" button 😝

Maybe, in the future, keep the PR as draft until the base has landed on main, to avoid such double-reviews?

Anyway, LGTM!

@atanmarko atanmarko merged commit faf8447 into main Feb 14, 2025
10 checks passed
@atanmarko atanmarko deleted the feat/refactor-configuration branch February 14, 2025 15:11
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.

Refactor configuration dependencies
3 participants