Skip to content

feat: enable clarity-serialization in cargo workspace #6326

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

Merged

Conversation

Jiloc
Copy link
Contributor

@Jiloc Jiloc commented Jul 23, 2025

Description

This PR moves clarity-serialization back to the workspace, as a pre-requisite for #6310

Applicable issues

  • fixes #

Additional info (benefits, drawbacks, caveats)

Checklist

  • Test coverage for new or modified code paths
  • Changelog is updated
  • Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
  • New clarity functions have corresponding PR in clarity-benchmarking repo
  • New integration test(s) added to bitcoin-tests.yml

@Jiloc Jiloc added this to the 3.2.0.0.1 milestone Jul 23, 2025
@Jiloc Jiloc self-assigned this Jul 23, 2025
@Jiloc Jiloc moved this to Status: 💻 In Progress in Stacks Core Eng Jul 23, 2025
@Jiloc Jiloc force-pushed the feat/move-clarity-serialization-to-root branch from ace54c3 to 9329fbc Compare July 23, 2025 15:08
@Jiloc Jiloc moved this from Status: 💻 In Progress to Status: In Review in Stacks Core Eng Jul 24, 2025
@Jiloc Jiloc added the clarity label Jul 25, 2025
@Jiloc Jiloc marked this pull request as ready for review July 25, 2025 09:54
@Jiloc Jiloc requested review from a team as code owners July 25, 2025 09:54
@Jiloc Jiloc requested review from kantai and fdefelici July 25, 2025 09:57
fdefelici
fdefelici previously approved these changes Jul 25, 2025
@aldur aldur requested a review from rdeioris July 30, 2025 14:44
Copy link
Contributor

@kantai kantai left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I think we should use rstest instead of test-case

@github-project-automation github-project-automation bot moved this from Status: In Review to Status: 💻 In Progress in Stacks Core Eng Aug 1, 2025
@Jiloc Jiloc added this pull request to the merge queue Aug 1, 2025
Merged via the queue into stacks-network:develop with commit 6d154cc Aug 1, 2025
3 of 4 checks passed
@Jiloc Jiloc deleted the feat/move-clarity-serialization-to-root branch August 1, 2025 09:23
@github-project-automation github-project-automation bot moved this from Status: 💻 In Progress to Status: ✅ Done in Stacks Core Eng Aug 1, 2025
Copy link

codecov bot commented Aug 1, 2025

Codecov Report

❌ Patch coverage is 80.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.27%. Comparing base (6168284) to head (e3bdb4c).
⚠️ Report is 61 commits behind head on develop.

Files with missing lines Patch % Lines
clarity-serialization/src/tests/representations.rs 75.00% 4 Missing ⚠️

❌ Your project status has failed because the head coverage (76.27%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (6168284) and HEAD (e3bdb4c). Click for more details.

HEAD has 7 uploads less than BASE
Flag BASE (6168284) HEAD (e3bdb4c)
133 126
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6326      +/-   ##
===========================================
- Coverage    81.45%   76.27%   -5.19%     
===========================================
  Files          540      552      +12     
  Lines       347695   351366    +3671     
===========================================
- Hits        283225   267990   -15235     
- Misses       64470    83376   +18906     
Files with missing lines Coverage Δ
clarity-serialization/src/errors.rs 100.00% <ø> (ø)
clarity-serialization/src/lib.rs 100.00% <ø> (ø)
clarity-serialization/src/representations.rs 90.47% <ø> (ø)
clarity-serialization/src/tests/mod.rs 100.00% <ø> (ø)
clarity-serialization/src/tests/types/mod.rs 86.11% <ø> (ø)
...ity-serialization/src/tests/types/serialization.rs 97.10% <100.00%> (ø)
...larity-serialization/src/tests/types/signatures.rs 0.00% <ø> (ø)
clarity-serialization/src/types/mod.rs 28.42% <ø> (ø)
clarity-serialization/src/types/serialization.rs 67.72% <ø> (ø)
clarity-serialization/src/types/signatures.rs 24.94% <ø> (ø)
... and 1 more

... and 226 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

github-actions bot commented Aug 9, 2025

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot added the locked label Aug 9, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants