Skip to content

refactor(NODE-7872): simplify check for null in findNull - #940

Merged
seanrmilligan merged 3 commits into
mainfrom
nul-fix
Oct 14, 2026
Merged

seanrmilligan merged 3 commits into
mainfrom
nul-fix

Conversation

@nbbeeken

Copy link
Copy Markdown
Contributor

Description

Summary of Changes

One less condition to check

Notes for Reviewers

What is the motivation for this change?

Less code, less problems

Double check the following

  • Lint is passing (npm run check:lint)
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@nbbeeken
nbbeeken requested a review from a team as a code owner September 30, 2026 02:31
Copilot AI balanced review requested due to automatic review settings September 30, 2026 02:31

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

The equivalent simplification preserves null detection and out-of-bounds handling for Uint8Array.

Review effort: Balanced
Findings: None

What changed in this PR

Simplifies null-terminator scanning in the on-demand BSON parser without changing behavior.

Changes:

  • Replaces the explicit bounds/value condition with a concise byte truthiness check.
File Description
src/​parser/​on_demand/​parse_to_elements.ts Simplifies findNull scanning logic.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@johnmtll johnmtll changed the title refactor: simplify check for null in findNull refactor(NODE-7872): simplify check for null in findNull Sep 30, 2026
@johnmtll

Copy link
Copy Markdown
Contributor

Thanks @nbbeeken! This has been assigned to https://jira.mongodb.org/browse/NODE-7872. Looks good to me. Will be addressed in the next triage session. 🙇

@dariakp dariakp added the tracked-in-jira There is a ticket in Mongo's Jira instance tracking this issue/PR label Oct 8, 2026
@seanrmilligan seanrmilligan self-assigned this Oct 13, 2026
@seanrmilligan seanrmilligan added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Oct 13, 2026
Comment thread src/parser/on_demand/parse_to_elements.ts
seanrmilligan
seanrmilligan previously approved these changes Oct 14, 2026
@seanrmilligan seanrmilligan added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Oct 14, 2026
seanrmilligan
seanrmilligan previously approved these changes Oct 14, 2026
Comment thread src/parser/on_demand/parse_to_elements.ts Outdated
tadjik1
tadjik1 previously approved these changes Oct 14, 2026

@tadjik1 tadjik1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice one, thanks @nbbeeken! 💯

Co-authored-by: Sergey Zelenov <[email protected]>
@seanrmilligan
seanrmilligan dismissed stale reviews from tadjik1 and themself via 466bb04 October 14, 2026 16:48
@seanrmilligan
seanrmilligan merged commit e96bf27 into main Oct 14, 2026
9 checks passed
@seanrmilligan
seanrmilligan deleted the nul-fix branch October 14, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

External Submission Team Review Needs review from team tracked-in-jira There is a ticket in Mongo's Jira instance tracking this issue/PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants