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

Add sections on zero-copy and exotic types to style guide #699

Merged
merged 2 commits into from
Jun 10, 2021

Conversation

sffc
Copy link
Member

@sffc sffc commented May 4, 2021

Fixes #523
See #652

@sffc sffc requested a review from a team as a code owner May 4, 2021 18:50
@sffc sffc mentioned this pull request May 4, 2021
7 tasks
@coveralls
Copy link

coveralls commented May 4, 2021

Pull Request Test Coverage Report for Build b12ff3d821ff7e3b06e2023758668aa417d939b3-PR-699

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 231 unchanged lines in 17 files lost coverage.
  • Overall coverage increased (+0.7%) to 74.188%

Files with Coverage Reduction New Missed Lines %
utils/fixed_decimal/src/lib.rs 1 50.0%
utils/zerovec/src/map/kv.rs 1 64.71%
components/datetime/src/fields/length.rs 2 87.5%
components/plurals/src/error.rs 3 20.0%
components/uniset/src/lib.rs 3 11.11%
experimental/provider_ppucd/src/error.rs 3 0%
components/plurals/src/rules/parser.rs 5 96.86%
components/datetime/src/error.rs 6 9.09%
components/datetime/src/pattern/error.rs 6 25.0%
components/plurals/src/operands.rs 7 91.95%
Totals Coverage Status
Change from base Build 478c4a901faca10195edce477b583ed31f8fdd1f: 0.7%
Covered Lines: 8134
Relevant Lines: 10964

💛 - Coveralls

Manishearth
Manishearth previously approved these changes May 4, 2021
zbraniecki
zbraniecki previously approved these changes May 5, 2021
gregtatum
gregtatum previously approved these changes May 5, 2021
Copy link
Member

@gregtatum gregtatum left a comment

Choose a reason for hiding this comment

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

The copy all seems reasonable to me, but I would hold off landing this until we have real examples in the code using the new ZeroCopy machinery. It seems a little early to fully require this, without any examples landed.

nordzilla
nordzilla previously approved these changes May 7, 2021
Copy link
Member

@nordzilla nordzilla left a comment

Choose a reason for hiding this comment

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

LGTM

@sffc
Copy link
Member Author

sffc commented May 8, 2021

Blocked on #652 per @gregtatum's comment above.

@sffc sffc added blocked A dependency must be resolved before this is actionable waiting-on-author PRs waiting for action from the author for >7 days labels May 8, 2021
@sffc sffc requested a review from Manishearth May 14, 2021 01:13
@sffc sffc removed the waiting-on-author PRs waiting for action from the author for >7 days label May 14, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #699 (8972a42) into main (478c4a9) will decrease coverage by 0.19%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #699      +/-   ##
==========================================
- Coverage   74.18%   73.99%   -0.20%     
==========================================
  Files         171      182      +11     
  Lines       10802    10870      +68     
==========================================
+ Hits         8014     8043      +29     
- Misses       2788     2827      +39     
Impacted Files Coverage Δ
components/datetime/src/error.rs 9.09% <0.00%> (-10.91%) ⬇️
components/datetime/src/pattern/error.rs 25.00% <0.00%> (-10.72%) ⬇️
components/plurals/src/rules/lexer.rs 88.17% <0.00%> (-1.94%) ⬇️
components/datetime/src/fields/symbols.rs 68.06% <0.00%> (-0.53%) ⬇️
utils/zerovec/src/map/kv.rs 64.70% <0.00%> (ø)
utils/zerovec/src/map/vecs.rs 85.10% <0.00%> (ø)
utils/fixed_decimal/src/lib.rs 50.00% <0.00%> (ø)
utils/zerovec/src/map/serde.rs 81.96% <0.00%> (ø)
components/plurals/src/operands.rs 91.95% <0.00%> (ø)
utils/zerovec/src/zerovec/serde.rs 88.05% <0.00%> (ø)
... and 118 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 478c4a9...8972a42. Read the comment docs.

@Manishearth Manishearth merged commit 61b7083 into unicode-org:main Jun 10, 2021
@sffc sffc deleted the styleguide branch July 26, 2021 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked A dependency must be resolved before this is actionable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exotic Types as fields of data provider structs
7 participants