Skip to content

Reducing complexity to 16#3338

Merged
RobinTail merged 7 commits intomasterfrom
pullfrog/3337-reduce-complexity-to-16
Apr 23, 2026
Merged

Reducing complexity to 16#3338
RobinTail merged 7 commits intomasterfrom
pullfrog/3337-reduce-complexity-to-16

Conversation

@pullfrog
Copy link
Copy Markdown
Contributor

@pullfrog pullfrog Bot commented Apr 23, 2026

Closes #3337

Lowers the ESLint complexity rule threshold from 18 to 16 for express-zod-api/src/*.ts.

Two functions exceeded the new limit at complexity 17 and were refactored by extracting helpers:

  • documentation-helpers.ts: extracted getLocation() from the depictRequestParams reduce callback
  • json-schema-helpers.ts: extracted mergeExamples() from flattenIO

Both extractions are pure refactors with no behavioral changes. All tests pass (864 unit tests + integration suites).

Pullfrog  | View workflow run | via Pullfrog | Using Claude Opus𝕏

- Lower ESLint complexity rule from 18 to 16 in eslint.config.js
- Extract getLocation() in documentation-helpers.ts to reduce reduce-callback complexity
- Extract mergeExamples() in json-schema-helpers.ts to reduce flattenIO complexity
@pullfrog pullfrog Bot requested a review from RobinTail April 23, 2026 10:07
@pullfrog pullfrog Bot mentioned this pull request Apr 23, 2026
@coveralls-official
Copy link
Copy Markdown

coveralls-official Bot commented Apr 23, 2026

Coverage Status

coverage: 100.0%. remained the same — pullfrog/3337-reduce-complexity-to-16 into master

Comment thread express-zod-api/src/json-schema-helpers.ts Outdated
@pullfrog
Copy link
Copy Markdown
Contributor Author

pullfrog Bot commented Apr 23, 2026

Addressed review feedback: marked mergeExamples as /** @internal */, exported it, and added 5 unit tests covering all branches (no examples, optional concat, optional init, required combinations, required init). All 869 tests pass.

Task list (3/3 completed)
  • Mark mergeExamples as @internal and export it
  • Add unit tests for mergeExamples
  • Commit, push, reply to review

Pullfrog  | View workflow run | via Pullfrog | Using Claude Opus𝕏

@RobinTail RobinTail added the refactoring The better way to achieve the same result label Apr 23, 2026
Comment thread express-zod-api/tests/json-schema-helpers.spec.ts Outdated
@pullfrog
Copy link
Copy Markdown
Contributor Author

pullfrog Bot commented Apr 23, 2026

Addressed review feedback: combined the two nearly identical mergeExamples tests ("should initialize examples when optional and flat has none" and "should handle required with no prior examples") into a single test.each([true, false]) parameterized test. All 37 tests pass.

Task list (3/3 completed)
  • Combine similar tests using test.each() per reviewer feedback
  • Run tests to verify changes
  • Commit, push, reply to review comment, resolve thread

Pullfrog  | View workflow run | via Pullfrog | Using Claude Opus𝕏

Comment thread express-zod-api/src/json-schema-helpers.ts Outdated
@pullfrog
Copy link
Copy Markdown
Contributor Author

pullfrog Bot commented Apr 23, 2026

Renamed the flat parameter to target in mergeExamples as requested. All 37 json-schema-helpers tests pass. Pushed and resolved the thread.

Task list (3/4 completed)
  • Rename flat parameter to target in mergeExamples function
  • Update all usages of flat inside mergeExamples body
  • Update test references if any use the parameter name
  • Run tests, commit, push, and reply to review comment

Pullfrog  | View workflow run | via Pullfrog | Using Claude Opus𝕏

Comment thread express-zod-api/src/documentation-helpers.ts Outdated
@pullfrog
Copy link
Copy Markdown
Contributor Author

pullfrog Bot commented Apr 23, 2026

Inlined isPathParam and isHeaderParam into getLocation as an if..return sequence per review feedback. Pushed and resolved the thread.

Task list (3/3 completed)
  • Inline isPathParam and isHeaderParam into getLocation as if..return..else..return
  • Run tests to verify correctness
  • Commit, push, reply to review, resolve thread

Pullfrog  | View workflow run | via Pullfrog | Using Claude Opus𝕏

Copy link
Copy Markdown
Owner

@RobinTail RobinTail left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏽

related to #3331 and #3329

@RobinTail RobinTail changed the title Reduce complexity threshold from 18 to 16 Reduce complexity to 16 Apr 23, 2026
@RobinTail RobinTail added the coverage Additional tests label Apr 23, 2026
Comment thread express-zod-api/src/documentation-helpers.ts Outdated
@pullfrog
Copy link
Copy Markdown
Contributor Author

pullfrog Bot commented Apr 23, 2026

Removed the redundant return undefined from getLocation in documentation-helpers.ts as requested. All 869 tests pass. Pushed and resolved the thread.

Task list (4/4 completed)
  • Read the relevant code and understand the reviewer's feedback
  • Remove the redundant return undefined statement
  • Run tests to verify the change
  • Commit, push, and reply to the review comment

Pullfrog  | View workflow run | via Pullfrog | Using Claude Opus𝕏

Comment thread express-zod-api/tests/json-schema-helpers.spec.ts Outdated
@pullfrog
Copy link
Copy Markdown
Contributor Author

pullfrog Bot commented Apr 23, 2026

Addressed review feedback: removed the explicit type annotation (which had {} triggering an eslint warning) and switched the assertion to .toHaveProperty("examples", [...]). All 37 tests in json-schema-helpers.spec.ts pass. Changes pushed and thread resolved.

Task list (4/4 completed)
  • Read the test code at the commented line and understand the feedback
  • Apply the fix: remove inline type annotation and use .toHaveProperty assertion
  • Run tests to verify changes
  • Commit, push, reply to review comment

Pullfrog  | View workflow run | via Pullfrog | Using Claude Opus𝕏

@RobinTail RobinTail changed the title Reduce complexity to 16 Reducing complexity to 16 Apr 23, 2026
@RobinTail RobinTail merged commit 63697a0 into master Apr 23, 2026
14 checks passed
@RobinTail RobinTail deleted the pullfrog/3337-reduce-complexity-to-16 branch April 23, 2026 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

coverage Additional tests refactoring The better way to achieve the same result

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reduce complexity to 16

1 participant