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

removing ledger backend module #1524

Merged
merged 6 commits into from
Jun 5, 2019
Merged

removing ledger backend module #1524

merged 6 commits into from
Jun 5, 2019

Conversation

gabor-aranyossy
Copy link
Contributor

@gabor-aranyossy gabor-aranyossy commented Jun 5, 2019

Removed the unnecessary ledger-backend module/abstraction.

Pull Request Checklist

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

@gabor-aranyossy gabor-aranyossy force-pushed the removing-ledger-backend branch from 827af11 to 554a078 Compare June 5, 2019 07:58
@gabor-aranyossy gabor-aranyossy force-pushed the removing-ledger-backend branch from 554a078 to 838c666 Compare June 5, 2019 08:06
@gabor-aranyossy gabor-aranyossy added the component/ledger Sandbox and Ledger API label Jun 5, 2019
@gabor-aranyossy gabor-aranyossy marked this pull request as ready for review June 5, 2019 08:36
@gabor-aranyossy gabor-aranyossy requested a review from gerolf-da June 5, 2019 08:36
@bitonic bitonic mentioned this pull request Jun 5, 2019
9 tasks
Copy link
Contributor

@gerolf-da gerolf-da left a comment

Choose a reason for hiding this comment

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

🎉 🎉 AWESOME 🎉 🎉
Let's see those intermediary layers fall fall fall!
Left a few comments, thanks!

},
inclusive = true // we need this to be inclusive otherwise the stream will be hanging until a new element from upstream arrives
)
.filter(_.offset.toLong < end.toLong)
.filter(_._1 < end.toLong)
}
// we MUST do the offset comparison BEFORE collecting only the accepted transactions,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update the comment with the changes (e.g. the concept of "accepted transaction" doesn't exist anymore).

@@ -23,7 +22,8 @@ import scala.collection.breakOut
trait TransactionConversion {

def acceptedToDomainFlat(
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename from acceptedToDomainFlat to something like ledgerEntryTxToDomainFlat.

filteredEvents,
domain.LedgerOffset.Absolute(trans.offset),
offset,
None
))
} else None
}

def acceptedToDomainTree(
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change the name.

gabor-aranyossy and others added 3 commits June 5, 2019 11:39
…ipant/state/v1/SubmitterInfo.scala

Co-Authored-By: Gary Verhaegen <gary.verhaegen@digitalasset.com>
@gabor-aranyossy
Copy link
Contributor Author

@garyverhaegen-da @gerolf-da your comments have been addressed

@mergify mergify bot merged commit 9cb6baa into master Jun 5, 2019
@mergify mergify bot deleted the removing-ledger-backend branch June 5, 2019 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/ledger Sandbox and Ledger API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants