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

Tiny refactor for shortenAddress inputs and docs #3004

Merged
merged 5 commits into from May 13, 2024

Conversation

gregfromstl
Copy link
Member

@gregfromstl gregfromstl commented May 11, 2024

PR-Codex overview

The focus of this PR is to update the shortenAddress function in thirdweb package to accept a string parameter instead of an Address.

Detailed summary

  • shortenAddress function now accepts a string parameter
  • Updated tests and examples to reflect the changes
  • Adjusted the function logic to handle string input and provide correct output

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Copy link

changeset-bot bot commented May 11, 2024

🦋 Changeset detected

Latest commit: 354c7a7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
thirdweb Patch
@thirdweb-dev/sdk Patch
@thirdweb-dev/cli Patch
@thirdweb-dev/react-core Patch
@thirdweb-dev/react Patch
@thirdweb-dev/unity-js-bridge Patch
@thirdweb-dev/wallets Patch
@thirdweb-dev/auth Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

codspeed-hq bot commented May 11, 2024

CodSpeed Performance Report

Merging #3004 will not alter performance

Comparing greg/shorten-address-2 (354c7a7) with main (7fcd60e)

Summary

✅ 9 untouched benchmarks

Copy link
Contributor

github-actions bot commented May 11, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 39.07 KB (0%) 782 ms (0%) 2.2 s (-0.41% 🔽) 2.9 s
thirdweb (cjs) 87.85 KB (-0.02% 🔽) 1.8 s (-0.02% 🔽) 7.5 s (+7.68% 🔺) 9.2 s
thirdweb (minimal + tree-shaking) 4.71 KB (0%) 95 ms (0%) 217 ms (+4.88% 🔺) 312 ms
thirdweb/chains (tree-shaking) 400 B (0%) 10 ms (0%) 185 ms (+262.4% 🔺) 195 ms
thirdweb/react (minimal + tree-shaking) 16.38 KB (0%) 328 ms (0%) 351 ms (-41.35% 🔽) 678 ms

Copy link

codecov bot commented May 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.55%. Comparing base (7fcd60e) to head (354c7a7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3004      +/-   ##
==========================================
- Coverage   64.55%   64.55%   -0.01%     
==========================================
  Files         759      759              
  Lines       53052    53052              
  Branches     3096     3093       -3     
==========================================
- Hits        34249    34248       -1     
- Misses      18138    18139       +1     
  Partials      665      665              
Flag Coverage Δ *Carryforward flag
legacy_packages 65.66% <ø> (ø) Carriedforward from 7fcd60e
packages 64.26% <100.00%> (-0.01%) ⬇️

*This pull request uses carry forward flags. Click here to find out more.

Files Coverage Δ
packages/thirdweb/src/utils/address.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

* ```
* @utils
*/
export function shortenAddress(address: Address, length = 4) {
export function shortenAddress(address: string, length = 4) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I meant it should leave 4 chars after the 0x on the first slice, like

0xabcd....1234

@gregfromstl gregfromstl added this pull request to the merge queue May 13, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 13, 2024
@gregfromstl gregfromstl added this pull request to the merge queue May 13, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks May 13, 2024
@gregfromstl gregfromstl added this pull request to the merge queue May 13, 2024
Merged via the queue into main with commit 145ff48 May 13, 2024
14 of 15 checks passed
@gregfromstl gregfromstl deleted the greg/shorten-address-2 branch May 13, 2024 21:26
@jnsdls jnsdls mentioned this pull request May 13, 2024
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.

None yet

2 participants