Skip to content

fix: preserve blank pagination parameters#195

Closed
kriptoburak wants to merge 4 commits into
7nohe:mainfrom
kriptoburak:codex/xquik-service-extraction-test
Closed

fix: preserve blank pagination parameters#195
kriptoburak wants to merge 4 commits into
7nohe:mainfrom
kriptoburak:codex/xquik-service-extraction-test

Conversation

@kriptoburak

@kriptoburak kriptoburak commented Jul 2, 2026

Copy link
Copy Markdown

Summary

Validation

  • pnpm build
  • pnpm lint
  • pnpm test -- --run (71 passed, 1 skipped)
  • git diff --check

The remaining Vercel authorization status requires maintainer approval.

@vercel

vercel Bot commented Jul 2, 2026

Copy link
Copy Markdown

@kriptoburak is attempting to deploy a commit to the Daiki Urata's projects Team on Vercel.

A member of the Team first needs to authorize it.

@kriptoburak kriptoburak changed the title test: cover Xquik service extraction fix: preserve blank pagination and cover Xquik extraction Jul 17, 2026
@kriptoburak

Copy link
Copy Markdown
Author

Published 1b0de43. The PR now includes the Xquik service-extraction coverage plus the independent #177 pagination fix. Local validation passes: build, Biome, and the full Vitest suite with 72 passed and 1 skipped. The remaining Vercel authorization status requires maintainer approval.

@7nohe 7nohe left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The empty string handling in safeParseNumber looks good and matches what was reported in #177, thanks. One change I'd like before merging, see the inline comment.

Comment thread tests/service.test.ts Outdated
@kriptoburak kriptoburak changed the title fix: preserve blank pagination and cover Xquik extraction fix: preserve blank pagination parameters Jul 18, 2026
@kriptoburak

Copy link
Copy Markdown
Author

Addressed the requested scope change in 198d0bd: removed the redundant service-specific test and kept only the #177 pagination fix with focused regression coverage. The review thread is resolved. Build, Biome, and all 71 tests pass. The only non-code status is the maintainer-controlled Vercel authorization check. Ready for re-review, @7nohe.

@kriptoburak

Copy link
Copy Markdown
Author

Closing this because the maintainer-requested scope now intentionally contains no Xquik placement. The accepted generic pagination fix should remain a separately scoped upstream contribution if the maintainer wants it, but this branch no longer serves the Xquik integration goal. Thanks for the detailed review.

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