split optional parent node path param into dedicated handlers #1487
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@notrab Hey thanks looks good. Looks like there's 2 follow-up issues that could be good to create. Details in comments. Please take the lead to merge when ready 👍
| description: "Indexer is within acceptable distance", | ||
| }, | ||
| 503: { | ||
| description: "Indexer is not within acceptable distance or indexing status unavailable", |
There was a problem hiding this comment.
Indexing status unavailable: this edge case seems better to represent as a HTTP 500 error. Shall we make a separate follow-up issue for this?
| */ | ||
| app.get( | ||
| "/:parentNode?", | ||
| "/:parentNode", |
There was a problem hiding this comment.
Hmm. It seems we should make a follow-up issue to make parentNode just another optional query param to the registrar actions API and not part of the path.
wdyt?
There was a problem hiding this comment.
@lightwalker-eth I agree, and it would actually mean most of this PR could be disregarded, and simpler without the duplicates.
Co-authored-by: lightwalker.eth <126201998+lightwalker-eth@users.noreply.github.com>
The optional path parameter pattern (
/:parentNode?) generated an invalid spec becausehono-openapicouldn't express an optional path parameter in a compliant way for OpenApi.Choose a PR template
Use the appropriate template for this PR:
Light PR
For small, low-risk changes (mechanical refactors, formatting, dependency bumps).
Substantial PR
For behavior changes, consumer impact, new concepts, or high-risk work.
If you're unsure, that suggests this PR is a Substantial PR.