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

Jump to bottom

Make ledger state dir optional to handle disable ledger option #1378

Merged
merged 9 commits into from Mar 23, 2023
Merged

Make ledger state dir optional to handle disable ledger option #1378

merged 9 commits into from Mar 23, 2023

Conversation

Cmdv
Copy link
Contributor

@Cmdv Cmdv commented Mar 15, 2023 •
edited

Description

Alternate effort to solving #1259

Add your description here, if it fixes a particular issue please provide a link to the issue.

Checklist

  • Commit sequence broadly makes sense
  • Commits have useful messages
  • New tests are added if needed and existing tests are updated
  • Any changes are noted in the changelog
  • Code is formatted with fourmolu (which can be run with scripts/fourmolize.sh
  • Self-reviewed the diff

Migrations

  • The pr causes a breaking change of type a,b or c
  • If there is a breaking change, the pr includes a database migration and/or a fix process for old values, so that upgrade is possible
  • Resyncing and running the migrations provided will result in the same database semantically

If there is a breaking change, especially a big one, please add a justification here. Please elaborate
more what the migration achieves, what it cannot achieve or why a migration is not possible.

kderme
kderme requested changes Mar 16, 2023
View reviewed changes
cardano-db-sync/src/Cardano/DbSync/Sync.hs Outdated Show resolved Hide resolved
cardano-db-sync/src/Cardano/DbSync/Api.hs Show resolved Hide resolved
cardano-db-sync/src/Cardano/DbSync/Database.hs Outdated Show resolved Hide resolved
@Cmdv Cmdv mentioned this pull request Mar 16, 2023
Closed
9 tasks
@Cmdv Cmdv marked this pull request as ready for review March 17, 2023 11:06
@Cmdv Cmdv requested a review from a team as a code owner March 17, 2023 11:06
kderme
kderme requested changes Mar 17, 2023
View reviewed changes
cardano-db-sync/app/cardano-db-sync.hs Outdated Show resolved Hide resolved
kderme
kderme reviewed Mar 20, 2023
View reviewed changes
Copy link
Contributor

@kderme kderme left a comment

Choose a reason for hiding this comment

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

Just a few comments added. Looks great otherwise!

cardano-db-sync/src/Cardano/DbSync/Api.hs Outdated Show resolved Hide resolved
cardano-db-sync/src/Cardano/DbSync/Api.hs Outdated Show resolved Hide resolved
kderme
kderme approved these changes Mar 20, 2023
View reviewed changes
Copy link
Contributor

@kderme kderme left a comment

Choose a reason for hiding this comment

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

Great work!

@kderme kderme merged commit 9b174eb into master Mar 23, 2023
4 of 7 checks passed
@iohk-bors iohk-bors bot deleted the 1259-state-directory-optional branch March 23, 2023 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kderme kderme kderme approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Cmdv @kderme