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

chore: Correct locations of various TSS protos #15780

Merged
merged 25 commits into from
Oct 9, 2024

Conversation

mhess-swl
Copy link
Member

Part of #14744

Signed-off-by: Matt Hess <[email protected]>
Copy link

codecov bot commented Oct 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.16%. Comparing base (872ca1a) to head (c4306fe).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             develop   #15780   +/-   ##
==========================================
  Coverage      58.16%   58.16%           
- Complexity     21628    21630    +2     
==========================================
  Files           2788     2788           
  Lines         109566   109566           
  Branches       11198    11198           
==========================================
+ Hits           63726    63731    +5     
+ Misses         41979    41975    -4     
+ Partials        3861     3860    -1     
Files with missing lines Coverage Δ
...om/hedera/node/app/tss/schemas/V0560TSSSchema.java 100.00% <ø> (ø)

... and 17 files with indirect coverage changes

Impacted file tree graph

Copy link

codacy-production bot commented Oct 3, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.00% (target: -1.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (872ca1a) 109383 67477 61.69%
Head commit (c4306fe) 109383 (+0) 67481 (+4) 61.69% (+0.00%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#15780) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

jsync-swirlds
jsync-swirlds previously approved these changes Oct 3, 2024
Copy link
Contributor

@edward-swirldslabs edward-swirldslabs left a comment

Choose a reason for hiding this comment

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

The proto files in this PR need to be deduplicated with the proto files in this commit: b19b06f

The comments on the proto files in this PR are more up to date and need to remain in the deduplicated files.

the package for the proto in the deduplicated files needs to reflect their final location. Likely not platform events since the transactions belong to an application level service.

Signed-off-by: Matt Hess <[email protected]>

# Conflicts:
#	hapi/hedera-protobufs/services/auxiliary/tss_message_transaction.proto
#	hapi/hedera-protobufs/services/auxiliary/tss_vote_transaction.proto
#	hapi/hedera-protobufs/services/transaction_body.proto
#	hapi/hedera-protobufs/services/tss_message.proto
#	hapi/hedera-protobufs/services/tss_message_transaction.proto
#	hapi/hedera-protobufs/services/tss_vote.proto
#	hapi/hedera-protobufs/services/tss_vote_transaction.proto
Signed-off-by: Matt Hess <[email protected]>
Copy link
Member

@jsync-swirlds jsync-swirlds left a comment

Choose a reason for hiding this comment

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

We still appear to have some confusion regarding the style guidelines for specification. Please consider commenting on the associated PR with suggested clarification to help make the guidelines easier to follow.

Copy link
Contributor

@thomas-swirlds-labs thomas-swirlds-labs left a comment

Choose a reason for hiding this comment

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

LGTM. tyvm! @mhess-swl

jsync-swirlds
jsync-swirlds previously approved these changes Oct 8, 2024
Copy link
Member

@jsync-swirlds jsync-swirlds left a comment

Choose a reason for hiding this comment

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

Much improved. Just a couple indentation issues.

Copy link
Member

@lukelee-sl lukelee-sl left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@thomas-swirlds-labs thomas-swirlds-labs left a comment

Choose a reason for hiding this comment

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

LGTM

@thomas-swirlds-labs thomas-swirlds-labs merged commit e31afe7 into develop Oct 9, 2024
50 checks passed
@thomas-swirlds-labs thomas-swirlds-labs deleted the tss-protobufs branch October 9, 2024 13:16
@thomas-swirlds-labs thomas-swirlds-labs linked an issue Oct 9, 2024 that may be closed by this pull request
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.

Create TSS related system transaction and state protobufs
8 participants