Conversation
There was a problem hiding this comment.
Code Review
This pull request splits a single glob pattern into two separate calls for '.js' and '.ts' files in 'jsdoc-region-tag', and updates the 'brace-expansion' override version to '5.0.7' in 'package.json'. Feedback highlights that 'brace-expansion' version '5.0.7' does not exist, which will cause installation failures, and that splitting the glob pattern is an inefficient workaround that should be reverted once the dependency issue is resolved.
| const sampleCandidates = [ | ||
| ...glob.sync(`${SAMPLES_DIRECTORY}/**/*.js`, {ignore: ['node_modules']}), | ||
| ...glob.sync(`${SAMPLES_DIRECTORY}/**/*.ts`, {ignore: ['node_modules']}), | ||
| ]; |
There was a problem hiding this comment.
Splitting the glob into two separate calls (**/*.js and **/*.ts) causes the directory tree to be traversed twice, which is inefficient. This workaround was likely introduced due to the broken brace-expansion dependency override. Reverting to the single glob pattern is preferred once the dependency issue is resolved.
const sampleCandidates = glob.sync(`${SAMPLES_DIRECTORY}/**/*.{js,ts}`, {
ignore: ['node_modules'],
});There was a problem hiding this comment.
I'll give this one a try.
This is part of the ongoing effort to reduce docs job failures.
b/545649275