Skip to content

docs: fix consumer and contributor docs and check Package.swift version - #46

Merged
tolgahan-arikan merged 7 commits into
masterfrom
docs-consumer-fixes
Oct 2, 2026
Merged

tolgahan-arikan merged 7 commits into
masterfrom
docs-consumer-fixes

Conversation

@tolgahan-arikan

@tolgahan-arikan tolgahan-arikan commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Documentation fixes, one release-tooling check, and the PR-review model change. Doc-comment text in Sources/ is corrected, with no code changes. scripts/verify.sh gains one README version check. Does not change Package.swift, the podspec, or versions.

Changes

  • Doc comments that contradicted signatures (corrections only; no new method descriptions, per AGENTS.md)

    • signMessage: network is the Network type, not a string.
    • Public createWallet: cited a nonexistent .ethereumEoa; now lists .ethereum (default) and .solana. .unknown(String) is a decoding fallback, not a creatable type.
    • Private useWallet: described a walletType parameter; it takes walletId.
    • Keccak256: the result is 0x plus 64 hex digits (66 characters).
    • signOut: the reference to startEmailAuth now includes sessionLifetimeSeconds.
  • README

    • Adds a Package.swift snippet (.product(name: "OMSWallet", package: "swift-sdk")).
    • Recommends Up to Next Minor, because 0.x minor releases can be breaking.
    • Documents SolanaNetwork and its values.
    • Adds .walletSelectionInFlight to error handling.
  • docs/error-contracts.md: adds OMS_WALLET_SELECTION_IN_FLIGHT.

  • TESTING.md: describes test files by area instead of a partial file list, and replaces a nonexistent example test name.

  • publishing.md: the README version text is required by verify.sh, not optional. The checklist also covers the new Package.swift example.

  • scripts/verify.sh: also fails when the README Package.swift example (from: "<version>") does not match the podspec version, so a release cannot leave it pinned to an old minor.

  • AGENTS.md: the utils description no longer mentions time helpers, which don't exist.

  • AGENTS.md Maintenance Matrix: the platform and Swift-version row now includes the podspec. It also notes that raising iOS or Swift/Xcode requirements raises the React Native SDK's supported minimum, which builds this SDK in compatibility-tests/expo-min (ci: build minimum and current Expo apps natively and fix docs 0xPolygon/oms-wallet-react-native-sdk#123).

  • Claude PR review (.github/workflows/claude-review.yml): both the automatic review and @claude replies now use Claude Sonnet 5.5 (claude-sonnet-5-5) instead of Opus 4.8. The branch merges master first to pick up its pinned claude-code-action (ci: pin claude-code-action to v1.0.237 by SHA (GHSA-8q5r-mmjf-575q) #47).

Testing

  • swift build passes
  • swift test passes (124 tests)
  • SDK demo app builds: not run, only doc comments changed
  • Trails Actions demo app builds: not run, only doc comments changed

Also:

  • The README Package.swift snippet resolves the 0.3.0 tag and builds in a scratch package.
  • scripts/generate-api.sh --check and scripts/test-generate-api.py pass. API.md is unchanged because the generator does not include doc-comment text.
  • Sources/ diffs are /// lines only.
  • The new verify.sh guard passes for the current 0.3.0 README. For a simulated 0.4.0 release with the prose and CocoaPods line bumped but the manifest example forgotten, it fails: README.md must use podspec version 0.4.0 in the Package.swift example.
  • git diff --check is clean.

Not included

  • The podspec uses swift_version "6.0" and verify.sh lints with --swift-version=6.0, while Package.swift needs tools 6.2. That is release configuration and belongs in a release PR.
  • AGENTS.md still says to avoid method descriptions in source, which contradicts the existing /// comments. Changing that rule needs a decision.

Related

Part of one documentation pass across the OMS Wallet SDKs and docs site:

Correct inaccurate doc comments, add a Package.swift install snippet,
Solana network and walletSelectionInFlight docs, and refresh the
TESTING, publishing, and AGENTS guidance.
@tolgahan-arikan tolgahan-arikan changed the title docs: fix consumer and contributor docs docs: fix consumer and contributor docs and check Package.swift version Sep 30, 2026
@tolgahan-arikan
tolgahan-arikan marked this pull request as ready for review October 1, 2026 12:00
@tolgahan-arikan
tolgahan-arikan requested a review from a team October 1, 2026 12:00
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-01T12:03:20.734805Z 655451b Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Claude finished @tolgahan-arikan's task in 1m 38s —— View job


Review: PR #46 — docs: fix consumer and contributor docs

  • Read PR diff and changed files
  • Verify doc-comment fixes against actual signatures
  • Check scripts/verify.sh new guard
  • Check README / docs correctness
  • Post review

Verdict: Looks good. Approve-equivalent. Every doc claim in this PR checks out against the source, and the new verify.sh guard is correct. This is a clean, well-scoped documentation pass with no code behavior changes.

Doc-comment fixes — all verified correct

Change Verified against source
signMessage network is Network WalletOperations.swift:11 — func signMessage(network: Network, …) ✓
createWallet lists .ethereum (default) / .solana + reference WalletModels.swift:3-6 (ethereum, solana, unknown(String)); WalletAuth.swift:776-778 (WalletType.ethereum default, reference: String? = nil) ✓
useWallet takes walletId WalletAuth.swift:836 — private func useWallet(walletId: String, …) ✓
Keccak256 returns 66 chars (0x + 64 hex) Keccak256.swift:82 — return "0x" + hash.map { … }.joined(). The old "64-character" comment was wrong. ✓
signOut references startEmailAuth(email:sessionLifetimeSeconds:) WalletAuth.swift:15-18 — matches the two-label signature ✓

README / docs

  • SolanaNetwork table matches Network.swift:3-5 (.devnet = "solana:devnet", .mainnet = "solana:mainnet"). The SolanaNetwork.mainnet vs Network.mainnet disambiguation is a genuinely useful clarification. ✓
  • .walletSelectionInFlight / OMS_WALLET_SELECTION_IN_FLIGHT exists (OMSWalletError.swift:12) and the README/error-contract description ("another action on the same PendingWalletSelection is still running") matches the actual semantics in PendingWalletSelection.swift:57-63. ✓
  • MIGRATION.md link target exists. ✓
  • "Up to Next Minor" guidance for a 0.x package is the correct SemVer-pre-1.0 recommendation and is consistent with the .upToNextMinor(from:) snippet. ✓

scripts/verify.sh guard

The new check is correct and the podspec version (0.3.0) matches the README Package.swift example, so it passes today:

if ! grep -Fq "from: \"$version\"" README.md; then

-Fq with the embedded version keeps it a fixed-string match, so the only from: "0.3.0" in the README (line 32) matches, and the unrelated from: Date()… on line 753 does not produce a false positive. The guard will correctly fail a release that bumps the podspec but forgets the manifest example.

TESTING.md / AGENTS.md

  • Replacement test name TestWalletCompleteEmailAuthUsesSessionLifetimeFromStart exists (MockWalletTests.swift:165). ✓
  • AGENTS.md Utils description now reads "encoding, hashing, request, byte, constants" — matches Sources/OMSWallet/Utils/ (Constants.swift present, no time helper). ✓

Against the repo-specific review checklist

  • No private key material / P-256 non-extractability: no code changes, untouched. ✓
  • No floating-point for token amounts: unchanged. ✓
  • @available on new public surfaces: no new public surfaces added. ✓
  • Test coverage: doc-only change; no new behavior to test. ✓
  • Shell examples / paths with spaces: the new verify.sh lines reference README.md only (no spaces); fine. ✓

Minor notes (non-blocking)

  • The PR description already flags the two known tensions and correctly defers them: (1) podspec swift_version "6.0" vs Package.swift tools 6.2 is release config, and (2) AGENTS.md's "avoid method descriptions in source" still contradicts the existing /// comments. The latter is worth a follow-up decision so the rule and the codebase stop disagreeing, but it's rightly out of scope here.

Nothing to change. 👍
· docs-consumer-fixes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 655451b6a4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Sources/OMSWallet/Clients/WalletAuth.swift Outdated
@tolgahan-arikan
tolgahan-arikan merged commit 429b957 into master Oct 2, 2026
4 checks passed
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.

2 participants