Skip to content

Conversation

@obycode
Copy link
Contributor

@obycode obycode commented Sep 3, 2025

Description

Builds off of #6403 (should review after that one is merged), adding to-ascii? support.

Applicable issues

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

@obycode obycode requested review from a team as code owners September 3, 2025 21:08
@obycode obycode moved this to Status: In Review in Stacks Core Eng Sep 3, 2025
@obycode obycode added this to the 3.3.0.0.0 milestone Sep 3, 2025
hstove
hstove previously approved these changes Sep 3, 2025
@obycode obycode linked an issue Sep 4, 2025 that may be closed by this pull request
@obycode obycode requested a review from kantai September 5, 2025 12:40
@obycode obycode marked this pull request as draft September 10, 2025 14:00
@obycode
Copy link
Contributor Author

obycode commented Sep 10, 2025

This has gotten too ugly. When #6403 merges, I will rebase this and re-open.

This new builtin function is added as part of Clarity 4, with epoch 3.3.

Closes stacks-network#6397
See stacksgov/sips/stacks-network#218
@obycode obycode marked this pull request as ready for review September 10, 2025 15:06
@obycode
Copy link
Contributor Author

obycode commented Sep 11, 2025

Fixed merge conflict in changelog.

@obycode obycode requested a review from hstove September 11, 2025 18:16
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.

LGTM, just had one comment

@obycode obycode requested a review from kantai September 11, 2025 19:37
kantai
kantai previously approved these changes Sep 11, 2025
hstove
hstove previously approved these changes Sep 15, 2025
Copy link
Contributor

@hstove hstove left a comment

Choose a reason for hiding this comment

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

LGTM! Just one nit about being a little more DRY but i'm happy to merge

@obycode obycode dismissed stale reviews from hstove and kantai via eb588c2 September 15, 2025 14:39
@obycode obycode requested review from hstove and kantai September 15, 2025 14:40
@obycode obycode enabled auto-merge September 15, 2025 15:51
@obycode obycode added this pull request to the merge queue Sep 15, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Sep 15, 2025
@obycode obycode added this pull request to the merge queue Sep 15, 2025
Merged via the queue into stacks-network:develop with commit 4747f97 Sep 15, 2025
1 of 2 checks passed
@obycode obycode deleted the feat/to-ascii branch September 15, 2025 19:24
@github-project-automation github-project-automation bot moved this from Status: In Review to Status: ✅ Done in Stacks Core Eng Sep 15, 2025
@codecov
Copy link

codecov bot commented Sep 15, 2025

Codecov Report

❌ Patch coverage is 92.85714% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.59%. Comparing base (c1a989f) to head (eb588c2).
⚠️ Report is 6 commits behind head on develop.

Files with missing lines Patch % Lines
clarity/src/vm/functions/conversions.rs 93.02% 3 Missing ⚠️
...y/src/vm/analysis/type_checker/v2_1/natives/mod.rs 84.61% 2 Missing ⚠️
clarity/src/vm/tests/conversions.rs 88.88% 1 Missing ⚠️

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

Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #6436       +/-   ##
============================================
+ Coverage    57.35%   74.59%   +17.24%     
============================================
  Files          558      559        +1     
  Lines       343239   343602      +363     
============================================
+ Hits        196873   256326    +59453     
+ Misses      146366    87276    -59090     
Files with missing lines Coverage Δ
clarity-types/src/types/signatures.rs 89.05% <ø> (+3.84%) ⬆️
clarity/src/vm/analysis/arithmetic_checker/mod.rs 92.48% <ø> (ø)
clarity/src/vm/analysis/read_only_checker/mod.rs 87.39% <ø> (+6.95%) ⬆️
.../src/vm/analysis/type_checker/v2_05/natives/mod.rs 89.17% <ø> (+0.87%) ⬆️
...c/vm/analysis/type_checker/v2_1/tests/contracts.rs 87.62% <ø> (+29.70%) ⬆️
clarity/src/vm/costs/cost_functions.rs 100.00% <100.00%> (ø)
clarity/src/vm/costs/costs_1.rs 100.00% <100.00%> (ø)
clarity/src/vm/costs/costs_2.rs 100.00% <100.00%> (ø)
clarity/src/vm/costs/costs_2_testnet.rs 100.00% <100.00%> (ø)
clarity/src/vm/costs/costs_3.rs 100.00% <100.00%> (+1.97%) ⬆️
... and 9 more

... and 394 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 c1a989f...eb588c2. 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.

@github-actions
Copy link

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 locked as resolved and limited conversation to collaborators Sep 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

Status: Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

[Clarity-4] to-ascii?

3 participants