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

go/consensus: use typed attributes in all services #4465

Merged
merged 3 commits into from Feb 15, 2022

Conversation

ptrus
Copy link
Member

@ptrus ptrus commented Feb 7, 2022

No description provided.

@ptrus ptrus added the s:ready-ci Status: ready for CI label Feb 7, 2022
@ptrus ptrus linked an issue Feb 7, 2022 that may be closed by this pull request
@ptrus ptrus force-pushed the ptrus/feature/typed-attributes branch 7 times, most recently from 81eb67d to c3bda78 Compare February 11, 2022 12:43
@codecov
Copy link

codecov bot commented Feb 11, 2022

Codecov Report

Merging #4465 (c3bda78) into master (b4de534) will decrease coverage by 0.08%.
The diff coverage is 79.43%.

❗ Current head c3bda78 differs from pull request most recent head ba0023b. Consider uploading reports for the commit ba0023b to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4465      +/-   ##
==========================================
- Coverage   68.87%   68.78%   -0.09%     
==========================================
  Files         421      423       +2     
  Lines       47174    47225      +51     
==========================================
- Hits        32489    32485       -4     
- Misses      10697    10744      +47     
- Partials     3988     3996       +8     
Impacted Files Coverage Δ
go/consensus/tendermint/roothash/roothash.go 67.16% <52.77%> (-0.60%) ⬇️
go/consensus/tendermint/scheduler/scheduler.go 58.02% <60.00%> (ø)
go/consensus/tendermint/registry/registry.go 66.18% <65.00%> (-2.42%) ⬇️
go/consensus/tendermint/apps/beacon/beacon.go 73.77% <75.00%> (ø)
go/consensus/tendermint/keymanager/keymanager.go 78.84% <75.00%> (ø)
go/consensus/tendermint/beacon/beacon.go 72.24% <80.00%> (ø)
go/beacon/api/api.go 55.55% <100.00%> (+5.55%) ⬆️
go/consensus/tendermint/api/api.go 84.46% <100.00%> (+0.30%) ⬆️
go/consensus/tendermint/api/context.go 88.63% <100.00%> (ø)
go/consensus/tendermint/apps/keymanager/genesis.go 59.61% <100.00%> (+1.61%) ⬆️
... and 52 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39e5f61...ba0023b. Read the comment docs.

@ptrus ptrus marked this pull request as ready for review February 11, 2022 13:50
Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

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

Since we're here, Tendermint 0.35 requires all events to be actual strings and doesn't support binary data anymore (so values would need to be Base64-encoded). It also requires no dashes in attribute keys. Maybe we should make the change now to avoid breaking changes later (also see #4427)? cc @abukosek

go/consensus/tendermint/api/api.go Outdated Show resolved Hide resolved
go/registry/api/api.go Outdated Show resolved Hide resolved
@ptrus
Copy link
Member Author

ptrus commented Feb 14, 2022

Maybe we should make the change now to avoid breaking changes later

Makes sense yes

@ptrus ptrus force-pushed the ptrus/feature/typed-attributes branch 5 times, most recently from 919b6aa to 320812e Compare February 15, 2022 09:53
go/consensus/tendermint/roothash/roothash.go Outdated Show resolved Hide resolved
go/consensus/tendermint/roothash/roothash.go Outdated Show resolved Hide resolved
go/registry/api/api.go Outdated Show resolved Hide resolved
go/roothash/api/api.go Outdated Show resolved Hide resolved
@ptrus ptrus force-pushed the ptrus/feature/typed-attributes branch from 320812e to 7221550 Compare February 15, 2022 10:06
@ptrus ptrus force-pushed the ptrus/feature/typed-attributes branch 2 times, most recently from 09195af to ea7973b Compare February 15, 2022 11:10
@ptrus ptrus force-pushed the ptrus/feature/typed-attributes branch from ea7973b to ba0023b Compare February 15, 2022 11:18
@ptrus ptrus enabled auto-merge February 15, 2022 11:46
@ptrus ptrus merged commit fcdb043 into master Feb 15, 2022
@ptrus ptrus deleted the ptrus/feature/typed-attributes branch February 15, 2022 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s:ready-ci Status: ready for CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use typed attributes in registry and roothash services
2 participants