Skip to content

Feature/explorer url builder#143

Open
laxjovial wants to merge 9 commits into
karagozemin:masterfrom
laxjovial:feature/explorer-url-builder
Open

Feature/explorer url builder#143
laxjovial wants to merge 9 commits into
karagozemin:masterfrom
laxjovial:feature/explorer-url-builder

Conversation

@laxjovial

Copy link
Copy Markdown
Contributor

closes #73

@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the karagoz's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Jun 29, 2026

Copy link
Copy Markdown

@laxjovial Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for the explorer URL builder work. I checked the latest commit, but this is not mergeable yet.

Commands run:

pnpm install --frozen-lockfile        # passed
pnpm --filter @oversync/sdk test     # failed
pnpm --filter @oversync/sdk build    # failed
git diff --check origin/master...HEAD # failed

Blocking failures:

  1. packages/sdk/src/ethereum/index.ts has a syntax error around claimOrder, so both tests and tsc fail:
packages/sdk/src/ethereum/index.ts:110:18: ERROR: Expected "=>" but found "("
src/ethereum/index.ts(110,3): error TS1434: Unexpected keyword or identifier.
  1. Existing Soroban order parsing tests now throw OverSyncError is not defined instead of the expected validation messages:
test/soroban-order.test.ts: 6 failed
expected ... to throw error including 'expected an object' but got 'OverSyncError is not defined'\n```\n\n3. `git diff --check origin/master...HEAD` reports trailing whitespace in:\n\n```\npackages/sdk/src/errors/index.ts:29\npackages/sdk/src/errors/index.ts:34\npackages/sdk/src/errors/index.ts:38\npackages/sdk/src/utils/explorer.ts:6\npackages/sdk/src/utils/explorer.ts:12\npackages/sdk/src/utils/explorer.ts:29\npackages/sdk/src/utils/explorer.ts:35\npackages/sdk/src/utils/explorer.ts:52\npackages/sdk/src/utils/explorer.ts:58\npackages/sdk/test/errors.test.ts:39\n```\n\nPlease fix these and re-run SDK test/build before I re-check.

@laxjovial

Copy link
Copy Markdown
Contributor Author

done . please kindly review

@karagozemin

Copy link
Copy Markdown
Owner

I checked the current PR state before reviewing code, and GitHub still reports this branch as CONFLICTING / DIRTY against current master. I cannot merge or give a final code approval while the branch cannot be merged.

Please rebase or merge latest master, resolve the conflicts, and push the updated branch. Once GitHub reports it as mergeable, I can run the relevant package tests and re-review.

…url-builder

# Conflicts:
#	packages/sdk/src/index.ts
@laxjovial

Copy link
Copy Markdown
Contributor Author

done please review

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for rebasing the explorer URL builder PR. I rechecked this under the karagozemin account.

Passing checks:

  • git diff --check origin/master...HEAD
  • the new packages/sdk/test/explorer.test.ts cases pass as part of the SDK suite

Still blocked:

SDK Error Normalization > throws VALIDATION_FAILED when wallet is missing in EthereumHTLCClient
expected 'UNKNOWN_ERROR' to be 'VALIDATION_FAILED'

Please either remove the unrelated error-normalization changes from this explorer PR, or fix the shared error mapping so the full SDK test suite passes. Then rerun:

  • pnpm --filter @oversync/sdk test
  • pnpm --filter @oversync/sdk build
  • git diff --check origin/master...HEAD

@laxjovial

Copy link
Copy Markdown
Contributor Author

changes have been made, please review

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for the update. I rechecked the merge ref from the maintainer account and the SDK checks pass:

git diff --check master...pr-143-merge
pnpm --filter @oversync/sdk test
pnpm --filter @oversync/sdk build

Result: SDK tests pass (67) and SDK build passes.

I cannot merge this exact PR yet because it is not scoped to #73. The explorer helper/test is present, but the diff also includes the SDK error normalization changes from #78:

packages/sdk/src/errors/index.ts
packages/sdk/src/ethereum/index.ts
packages/sdk/src/soroban/index.ts
packages/sdk/test/errors.test.ts

Please narrow this branch to the explorer URL helper/tests only, or consolidate the related SDK changes into a single PR that explicitly closes #73 and #78. Once the scope is clean, the checks look good.

@laxjovial

Copy link
Copy Markdown
Contributor Author

All three of my PR branches are now cleanly scoped to their respective issues

@laxjovial

Copy link
Copy Markdown
Contributor Author

@karagozemin

@karagozemin

Copy link
Copy Markdown
Owner

Thanks! ✅ Close, but the build fails in packages/sdk:

src/index.ts(5,15): error TS2307: Cannot find module './errors/index.js'

The errors/index.ts barrel referenced from src/index.ts is missing from the PR. Please add it (or correct the import path) so pnpm build passes, then I'll merge. 🙏

@laxjovial

Copy link
Copy Markdown
Contributor Author

@karagozemin come and see

@karagozemin

karagozemin commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Rechecked: thanks for the updates. However this PR now has merge conflicts with main, so I cannot merge it as-is. Could you rebase/merge the latest main, resolve the conflicts and push? I will take another look and merge afterwards.

@laxjovial

Copy link
Copy Markdown
Contributor Author

Done

@laxjovial

Copy link
Copy Markdown
Contributor Author

Uploading IMG_8506.png…

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.

[Wave 7 4/10] Explorer URL builder tests for Sepolia and Stellar testnet

2 participants