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.

Sign up for GitHub

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

CORE-17972: APIs for Send and Receive Transaction #1346

Merged
merged 6 commits into from
Nov 21, 2023
Merged

CORE-17972: APIs for Send and Receive Transaction #1346

merged 6 commits into from
Nov 21, 2023

Conversation

jennyang-r3
Copy link
Contributor

Added APIs for sending transaction and receiving transaction.

github-actions[bot]
github-actions bot previously requested changes Nov 16, 2023
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

PR title failed to match regex -> ^((CORDA|EG|ENT|INFRA|CORE|DOC|ES)-\d+)(.*)

@jennyang-r3 jennyang-r3 changed the title Jennyang/core 17972 CORE-17972: APIs for Send and Receive Transaction Nov 16, 2023
@github-actions github-actions bot dismissed their stale review November 16, 2023 17:04

All good!

@corda-jenkins-ci02
Copy link
Contributor

corda-jenkins-ci02 bot commented Nov 16, 2023 •
edited
Loading

Jenkins build for PR 1346 build 7

Build Successful:
Jar artifact version produced by this PR: 5.2.0.4-alpha-1700500157080

@corda-jenkins-ci02
Copy link
Contributor

corda-jenkins-ci02 bot commented Nov 16, 2023 •
edited
Loading

Scanning for breaking API changes introduced by this PR

Scan Succeeded

@corda-jenkins-ci02
Copy link
Contributor

corda-jenkins-ci02 bot commented Nov 16, 2023 •
edited
Loading

Non-blocking downstream job failed for corda-non-functional-test

https://ci02.dev.r3.com/job/Corda5/job/corda-api-compatibility/job/PR-1346/5/ has failed for PR 1346 build 5

Please investigate if your changes may have broken compilation on https://github.com/corda/corda-non-functional-test

@corda-jenkins-ci02
Copy link
Contributor

corda-jenkins-ci02 bot commented Nov 16, 2023 •
edited
Loading

Non-blocking downstream job failed for corda-e2e-test

https://ci02.dev.r3.com/job/Corda5/job/corda-api-compatibility/job/PR-1346/5/ has failed for PR 1346 build 5

Please investigate if your changes may have broken compilation on https://github.com/corda/corda-e2e-tests

@jennyang-r3 jennyang-r3 marked this pull request as ready for review November 16, 2023 17:43
@jennyang-r3 jennyang-r3 requested a review from a team as a code owner November 16, 2023 17:43
lankydan
lankydan reviewed Nov 20, 2023
View reviewed changes
ledger/ledger-utxo/scans/corda-ledger-utxo-5.2.0.yaml
@@ -421,6 +421,17 @@ net.corda.v5.ledger.utxo.UtxoLedgerService:
annotation:
- NotNull
type: net.corda.v5.ledger.utxo.transaction.UtxoTransactionValidator
receiveTransaction:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@blsemo do you have any information on what this file is for because this is new to me?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is not that related to this PR? #1274

Copy link
Contributor Author

@jennyang-r3 jennyang-r3 Nov 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

this was generated when I ran ./gradlew cementApi to be able to raise breaking changes to PR, it looks like we now must run it to raise it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What does it define as breaking changes? 🤔 We didn't break anything...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

adding new APIs is breaking change it looks like.

Copy link
Contributor

Choose a reason for hiding this comment

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

Adding APIs needs to be added to the list of things not to break, so you need to regenerate.

lankydan
lankydan reviewed Nov 20, 2023
View reviewed changes
ledger/ledger-utxo/src/main/java/net/corda/v5/ledger/utxo/UtxoLedgerService.java Outdated Show resolved Hide resolved
ledger/ledger-utxo/src/main/java/net/corda/v5/ledger/utxo/UtxoLedgerService.java Outdated Show resolved Hide resolved
ledger/ledger-utxo/src/main/java/net/corda/v5/ledger/utxo/UtxoLedgerService.java Outdated Show resolved Hide resolved
ledger/ledger-utxo/src/main/java/net/corda/v5/ledger/utxo/UtxoLedgerService.java Outdated Show resolved Hide resolved
ledger/ledger-utxo/src/main/java/net/corda/v5/ledger/utxo/UtxoLedgerService.java Outdated Show resolved Hide resolved
ledger/ledger-utxo/src/main/java/net/corda/v5/ledger/utxo/UtxoLedgerService.java Outdated Show resolved Hide resolved
ledger/ledger-utxo/src/main/java/net/corda/v5/ledger/utxo/UtxoLedgerService.java Outdated Show resolved Hide resolved
ledger/ledger-utxo/src/main/java/net/corda/v5/ledger/utxo/UtxoLedgerService.java Outdated Show resolved Hide resolved
ledger/ledger-utxo/src/main/java/net/corda/v5/ledger/utxo/UtxoLedgerService.java Outdated Show resolved Hide resolved
blsemo
blsemo previously approved these changes Nov 20, 2023
View reviewed changes
Copy link
Contributor

@blsemo blsemo left a comment

Choose a reason for hiding this comment

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

LGTM

lankydan
lankydan previously approved these changes Nov 20, 2023
View reviewed changes
@jennyang-r3 jennyang-r3 dismissed stale reviews from lankydan and blsemo via 80d209a November 20, 2023 17:08
@sonarqubecloud SonarQubeCloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

blsemo
blsemo approved these changes Nov 21, 2023
View reviewed changes
Copy link
Contributor

@blsemo blsemo left a comment

Choose a reason for hiding this comment

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

LGTM

ledger/ledger-utxo/scans/corda-ledger-utxo-5.2.0.yaml
@@ -421,6 +421,17 @@ net.corda.v5.ledger.utxo.UtxoLedgerService:
annotation:
- NotNull
type: net.corda.v5.ledger.utxo.transaction.UtxoTransactionValidator
receiveTransaction:
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding APIs needs to be added to the list of things not to break, so you need to regenerate.

@jennyang-r3 jennyang-r3 merged commit 6911079 into release/os/5.2 Nov 21, 2023
4 checks passed
@jennyang-r3 jennyang-r3 deleted the jennyang/CORE-17972 branch November 21, 2023 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nkovacsx nkovacsx nkovacsx left review comments

@vlajos vlajos vlajos left review comments

@lankydan lankydan lankydan approved these changes

@blsemo blsemo blsemo approved these changes

@github-actions github-actions[bot] github-actions[bot] left review comments

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

Successfully merging this pull request may close these issues.
5 participants
@jennyang-r3 @blsemo @vlajos @lankydan @nkovacsx