Skip to content

Comments

Add scripts/validate-structure.js validation script#3

Merged
benjie merged 2 commits intographql:mainfrom
magicmark:add-validation-script
Feb 16, 2026
Merged

Add scripts/validate-structure.js validation script#3
benjie merged 2 commits intographql:mainfrom
magicmark:add-validation-script

Conversation

@magicmark
Copy link
Contributor

e.g.

$ npm run test:structure

> test:structure
> find . -type d -name 'GAP-*' -exec ./scripts/validate-structure.js {} \;

GAP-1234: metadata.yml validation failed:

must have required property 'id'

I didn't add a test for this - i'm happy to follow up and do so if anyone thinks it's valuable enough.

@magicmark magicmark changed the title Add scripts/validate-structure.js validation hook Add scripts/validate-structure.js validation script Feb 3, 2026
@magicmark magicmark force-pushed the add-validation-script branch 2 times, most recently from 3028209 to ca12350 Compare February 3, 2026 16:25
@magicmark magicmark force-pushed the add-validation-script branch from ca12350 to f1572ca Compare February 3, 2026 16:27
Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

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

Looking good; I think we should replace the find usage but other than that good to merge I think!

package.json Outdated
"test:format": "prettier --check . || npm run suggest:format",
"test:spelling": "cspell \"spec/**/*.md\" README.md LICENSE.md"
"test:spelling": "cspell \"spec/**/*.md\" README.md LICENSE.md",
"test:structure": "find . -type d -name 'GAP-*' -exec ./scripts/validate-structure.js {} \\;"
Copy link
Member

Choose a reason for hiding this comment

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

Not ideal; this would find GAP-* (unlikely currently) folders inside node_modules for example. Maybe just incorporate the finding into the validate-structure.js script? Note glob is built in to Node.js now:

https://nodejs.org/api/fs.html#fspromisesglobpattern-options

Copy link
Contributor Author

@magicmark magicmark Feb 12, 2026

Choose a reason for hiding this comment

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

Alterantively, could add -depth 1 to guard against nested GAP- directories (I had that originally but removed for some dumb reason)

I did it his way b/c i like the idea of the script not having the responsibility of also discovering which folders to run on, because it means we can throw a test directory at it and easily add a test suite without code changes. Then again, maybe yagni.

wdyt? happy to just go ahead and make the script use glob and discover dirs itself to keep it all self contained, I don't feel too strongly either way here.

Copy link
Contributor Author

@magicmark magicmark Feb 16, 2026

Choose a reason for hiding this comment

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

made the alt version here using glob #5 -- otherwise identical logic -- approve / merge whichever pr sparks more joy :)

(rip no stacked prs yet, and I didn't want to PR against my own fork)

The find command now uses -maxdepth 1 to restrict the search to the top-level
directory only, preventing unintended matches in subdirectories like node_modules.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

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

We can optimize the tooling later!

@benjie benjie merged commit be0d18e into graphql:main Feb 16, 2026
1 check passed
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