Skip to content

[CDX-449] Remove numResults=20 parameter from URL#242

Open
esezen wants to merge 3 commits intomainfrom
cdx-449-plp-ui-strip-default-numresults20-from-serialized-urls
Open

[CDX-449] Remove numResults=20 parameter from URL#242
esezen wants to merge 3 commits intomainfrom
cdx-449-plp-ui-strip-default-numresults20-from-serialized-urls

Conversation

@esezen
Copy link
Copy Markdown
Contributor

@esezen esezen commented Apr 24, 2026

Related: #231

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no API changes)
  • Documentation content changes
  • TypeScript type definitions update
  • Other... Please describe:

Copilot AI review requested due to automatic review settings April 24, 2026 19:38
@esezen esezen requested a review from a team as a code owner April 24, 2026 19:38
constructor-claude-bedrock[bot]

This comment was marked as outdated.

This comment was marked as outdated.

@esezen esezen requested a review from Copilot April 24, 2026 19:56
Copy link
Copy Markdown

@constructor-claude-bedrock constructor-claude-bedrock Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This PR fixes a bug where numResults=20 was unnecessarily appended to URLs when the value matched the library default, mirroring the existing page=1 suppression behavior. The implementation is clean and well-tested.

Inline comments: 2 discussions added

Overall Assessment: ⚠️ Needs Work

Comment thread src/utils/urlHelpers.ts
Comment thread spec/utils/urlHelpers.test.tsx
Copy link
Copy Markdown
Contributor Author

@esezen esezen left a comment

Choose a reason for hiding this comment

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

Self review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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