search

mit-dci / opencbdc-tx

A transaction processor for a hypothetical, general-purpose, central bank digital currency
Other
898 stars 198 forks source link

Sharded_DB branch #240

Closed natecable closed 10 months ago

maurermi commented 10 months ago

Hi @natecable, thanks for PRing this looking forward to reviewing it. I am interested in seeing what OracleDB makes possible in OpenCBDC. Haven't had a chance to look in depth yet, but here are a couple quick notes that would help us with the review process.

  1. We use a DCO check, which requires all commits to contain a "Signed-off-by" line. This can be done automatically using git commit -s.
  2. It would be great if some of these commits were squashed together where possible, specifically those that correspond to the same proposed change. We generally aim to have each commit cover one feature change (or a small set of feature changes) in the code, and look to give each commit a description of what the change is (typically a high-level one line description, followed by a more in depth description when necessary). This helps us understand all the changes proposed when we conduct a review, and in the future helps us understand the code history.

These could probably be tackled at the same time, and would help the process quite a bit! Happy to answer any questions as well.

natecable commented 10 months ago

Sorry about that! These changes were accidentally submitted to the original branch as well as my team’s personal fork. My bad!

On Sun, Nov 19, 2023 at 5:02 PM Michael Maurer @.***> wrote:

Hi @natecable https://github.com/natecable, thanks for PRing this looking forward to reviewing it. I am interested in seeing what OracleDB makes possible in OpenCBDC. Haven't had a chance to look in depth yet, but here are a couple quick notes that would help us with the review process.

  1. We use a DCO check, which requires all commits to contain a "Signed-off-by" line. This can be done automatically using git commit -s.
  2. It would be great if some of these commits were squashed together where possible, specifically those that correspond to the same proposed change. We generally aim to have each commit cover one feature change (or a small set of feature changes) in the code, and look to give each commit a description of what the change is (typically a high-level one line description, followed by a more in depth description when necessary). This helps us understand all the changes proposed when we conduct a review, and in the future helps us understand the code history.

These could probably be tackled at the same time, and would help the process quite a bit! Happy to answer any questions as well.

— Reply to this email directly, view it on GitHub https://github.com/mit-dci/opencbdc-tx/pull/240#issuecomment-1817991302, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVLU535NAJPHNVAH2L6QAHDYFJ6V3AVCNFSM6AAAAAA7R7XMNWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJXHE4TCMZQGI . You are receiving this because you were mentioned.Message ID: @.***>

maurermi commented 10 months ago

@natecable No worries!

HalosGhost commented 10 months ago

@natecable I'm going to close this now, but feel free to reopen if you'd like it to be considered for inclusion upstream!