-
Notifications
You must be signed in to change notification settings - Fork 305
metadata-3directories #1441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
metadata-3directories #1441
Conversation
cpengilly
commented
Mar 4, 2025
- notices, connect, get-started
✅ Deploy Preview for docs-optimism ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
📝 WalkthroughWalkthroughThe changes introduced in this pull request affect multiple parts of the project. In the Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CLI as Metadata CLI
participant Batch as metadata-batch-cli.ts
participant Analyzer as metadata-analyzer.ts
participant Manager as metadata-manager.ts
Dev->>CLI: Execute "metadata-batch-cli:dry" command
CLI->>Batch: Invoke file processing (processFiles)
Batch->>Analyzer: Analyze file content (analyzeContent)
Analyzer->>Analyzer: Call detectCategories(content, filepath)
Analyzer-->>Batch: Return metadata analysis result
Batch->>Manager: (Optional) Update metadata file via updateMetadataFile
Batch-->>Dev: Log summary and error details
Possibly related PRs
Suggested labels
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
utils/metadata-analyzer.ts (1)
253-400: 🛠️ Refactor suggestionRemove unused helper functions
The
detectCategoriesfunction no longer calls the helper functionsdetectStackCategories,detectOperatorCategories,detectAppDeveloperCategories, ordetectCommonCategories, but these functions are still defined (lines 257-400) and exported for testing (lines 590-593).Either remove these unused functions or update
detectCategoriesto use them:// Remove these unused functions -function detectStackCategories(filepath: string, content: string): Set<string> { - // ...function body... -} -function detectOperatorCategories(filepath: string, content: string): Set<string> { - // ...function body... -} -function detectAppDeveloperCategories(filepath: string, content: string): Set<string> { - // ...function body... -} -function detectCommonCategories(content: string, filepath: string): Set<string> { - // ...function body... -} // And update the export for testing export const testing = { DEFAULT_CONFIG, - detectStackCategories, - detectOperatorCategories, - detectAppDeveloperCategories, - detectCommonCategories, detectCategories, detectContentType, isLandingPage, getLandingPageCategories, MetadataAnalysisError };Also applies to: 590-593
🧹 Nitpick comments (8)
pages/notices/custom-gas-tokens-deprecation.mdx (6)
18-18: Consider updating heading to sentence case.According to the coding guidelines, H1, H2, and H3 headers should use sentence case, capitalizing only the first word while preserving proper nouns.
-## Deprecation of Custom Gas Tokens +## Deprecation of custom gas tokens
27-27: Consider updating heading to sentence case.According to the coding guidelines, H1, H2, and H3 headers should use sentence case.
-## Highlights +## Highlights
33-33: Consider updating heading to sentence case.According to the coding guidelines, H1, H2, and H3 headers should use sentence case.
-## Options for new chains +## Options for new chains
43-43: Consider updating heading to sentence case.According to the coding guidelines, H1, H2, and H3 headers should use sentence case.
-## Options for existing chains +## Options for existing chains
51-51: Consider updating heading to sentence case.According to the coding guidelines, H1, H2, and H3 headers should use sentence case.
-## Risks of continued use of Custom Gas Tokens +## Risks of continued use of custom gas tokens
63-63: Consider updating heading to sentence case.According to the coding guidelines, H1, H2, and H3 headers should use sentence case.
-### Need help? +### Need help?pages/connect/resources/glossary.mdx (1)
2-5: Clarify title capitalizationThe title in the frontmatter is lowercase "glossary" but the main heading on line 23 is capitalized "Glossary". Consider standardizing the capitalization.
-title: glossary +title: Glossaryutils/metadata-analyzer.ts (1)
405-425: Consider using Set to prevent duplicate categoriesThe
detectCategoriesfunction now returns an array directly from the Set, which is good for ensuring uniqueness. However, if additional categories are added without using the Set (e.g., direct array manipulation), duplicates could be introduced.Consider maintaining the use of the Set throughout and only converting to array at the end:
function detectCategories(content: string, filepath: string, detectionLog: string[]): string[] { const categories = new Set<string>() // Add categories based on content keywords if (content.toLowerCase().includes('mainnet')) { categories.add('mainnet') } if (content.toLowerCase().includes('testnet') || content.toLowerCase().includes('devnet')) { categories.add('testnet') } // Add categories based on filepath if (filepath.includes('/security/')) { categories.add('security') } if (filepath.includes('/fault-proofs/')) { categories.add('protocol') } if (filepath.includes('/interop/')) { categories.add('protocol') } if (filepath.includes('/transactions/')) { categories.add('protocol') } // Always include protocol for stack documentation if (filepath.includes('/stack/')) { categories.add('protocol') } + + // Ensure all categories are valid before adding + const validCategories = Array.from(categories).filter(category => + VALID_CATEGORIES.includes(category) + ) - detectionLog.push(`Detected categories: ${Array.from(categories).join(', ')}`) - return Array.from(categories) + detectionLog.push(`Detected categories: ${validCategories.join(', ')}`) + return validCategories }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
package.json(1 hunks)pages/connect/contribute.mdx(1 hunks)pages/connect/contribute/docs-contribute.mdx(1 hunks)pages/connect/contribute/stack-contribute.mdx(1 hunks)pages/connect/contribute/style-guide.mdx(1 hunks)pages/connect/resources.mdx(1 hunks)pages/connect/resources/glossary.mdx(1 hunks)pages/get-started/interop.mdx(1 hunks)pages/get-started/op-stack.mdx(1 hunks)pages/get-started/superchain.mdx(1 hunks)pages/notices/custom-gas-tokens-deprecation.mdx(1 hunks)pages/notices/holocene-changes.mdx(1 hunks)pages/notices/pectra-changes.mdx(1 hunks)pages/notices/sdk-deprecation.mdx(1 hunks)utils/metadata-analyzer.ts(2 hunks)utils/metadata-batch-cli.ts(5 hunks)utils/metadata-manager.ts(3 hunks)utils/types/metadata-types.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- pages/notices/holocene-changes.mdx
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.mdx`: "ALWAYS review Markdown content THOROUGHLY with ...
**/*.mdx: "ALWAYS review Markdown content THOROUGHLY with the following criteria:
- First, check the frontmatter section at the top of the file:
- For regular pages, ensure ALL these fields are present and not empty:
--- title: [non-empty] lang: [non-empty] description: [non-empty] topic: [non-empty] personas: [non-empty array] categories: [non-empty array] content_type: [valid type] ---
- For landing pages (index.mdx or files with ), only these fields are required:
--- title: [non-empty] lang: [non-empty] description: [non-empty] topic: [non-empty] ---
- If any required fields are missing or empty, comment:
'This file appears to be missing required metadata. You can fix this by running:Review the changes, then run without :dry to apply them.'pnpm metadata-batch-cli:dry "path/to/this/file.mdx"- Use proper nouns in place of personal pronouns like 'We' and 'Our' to maintain consistency in communal documentation.
- Avoid gender-specific language and use the imperative form.
- Monitor capitalization for emphasis. Avoid using all caps, italics, or bold for emphasis.
- Ensure proper nouns are capitalized in sentences.
- Apply the Oxford comma.
- Use proper title case for buttons, tab names, page names, and links. Sentence case should be used for body content and short phrases, even in links.
- Use correct spelling and grammar at all times (IMPORTANT).
- For H1, H2, and H3 headers:
- Use sentence case, capitalizing only the first word.
- Preserve the capitalization of proper nouns, technical terms, and acronyms as defined in the 'nouns.txt' file located in the root directory of the project.
- Do not automatically lowercase words that appear in the 'nouns.txt' file, regardless of their position in the header.
- Flag any headers that seem to inconsistently apply these rules for manual review.
- When reviewing capitalization, always refer to the 'nouns.txt' file for the correct capitalization of proper nouns and technical terms specific to the project.
"
pages/connect/resources.mdxpages/connect/contribute.mdxpages/notices/sdk-deprecation.mdxpages/get-started/interop.mdxpages/get-started/op-stack.mdxpages/get-started/superchain.mdxpages/connect/contribute/stack-contribute.mdxpages/connect/contribute/docs-contribute.mdxpages/notices/custom-gas-tokens-deprecation.mdxpages/connect/contribute/style-guide.mdxpages/notices/pectra-changes.mdxpages/connect/resources/glossary.mdx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - docs-optimism
- GitHub Check: Header rules - docs-optimism
- GitHub Check: Pages changed - docs-optimism
🔇 Additional comments (33)
pages/get-started/interop.mdx (1)
2-14: Metadata fields all present and correctly formatted.The updated metadata contains all the required fields for regular pages, including title, description, lang, content_type, topic, personas, categories, and is_imported_content. The addition of structured metadata will help with content organization and discoverability.
pages/get-started/op-stack.mdx (1)
4-16: Metadata fields all present and correctly formatted.The updated metadata contains all the required fields for regular pages, including title, description, lang, content_type, topic, personas, categories, and is_imported_content. The persona list appropriately includes all relevant roles for the OP Stack content.
pages/notices/custom-gas-tokens-deprecation.mdx (1)
2-15: Metadata fields all present and correctly formatted.The updated metadata contains all the required fields for regular pages. The description has been correctly formatted using the YAML multi-line string syntax (>-). The categories appropriately reflect the security nature of the content.
utils/types/metadata-types.ts (1)
118-123: Appropriate addition of the suggestions property to the MetadataResult interface.The suggestions property is well-structured and aligns with the existing metadata fields, making it easy to provide suggested values for content_type, categories, topic, and personas. This enhancement will help improve the metadata management capabilities in the application.
pages/connect/resources.mdx (2)
1-18: Frontmatter metadata compliance
The frontmatter includes all required fields—title, description, lang, content_type, topic, personas, and categories—and they are not empty. The block-style description improves readability and adheres to the documentation guidelines.
20-29: MDX content structure review
The MDX content uses standard components (CardandCards) appropriately and clearly defines the page structure. The separation of metadata and content is clean and consistent with project conventions.package.json (1)
17-17: Updated script for metadata-batch-cli:dry
The updated command now uses direct Node.js execution with TypeScript support (via thets-node/esmloader) and suppresses warnings by settingNODE_NO_WARNINGS=1. This change streamlines script execution and aligns with the intended improvements.pages/connect/contribute.mdx (2)
1-18: Frontmatter metadata compliance for Contribute page
All required frontmatter fields are present and correctly formatted. The block-style description and properly formatted arrays for personas and categories comply with the documentation standards.
20-31: Content structure and component utilization
The MDX file clearly separates metadata from content. The use of imported components (e.g.,CardandCards) to organize content supports a maintainable and cohesive layout.pages/connect/contribute/stack-contribute.mdx (1)
1-16: Frontmatter metadata compliance for Stack Contribute page
The metadata section includes all required fields in accordance with the guidelines. The new fields—content_type, topic, personas, categories, and is_imported_content—are correctly formatted, ensuring consistency with other documentation pages.pages/notices/sdk-deprecation.mdx (1)
1-16: Frontmatter metadata compliance for SDK deprecation notice
The metadata accurately reflects the updated purpose of the page, including a focused title, detailed block-style description, and new metadata fields that guide developers on the migration from the Optimism SDK toviem. All required fields are properly provided.pages/get-started/superchain.mdx (1)
3-16: Metadata structure follows requirements correctly.The frontmatter metadata has been properly updated with all required fields:
- The description is now properly quoted
- content_type is specified as "guide"
- topic is specified as "superchain"
- personas includes four relevant roles
- categories includes three appropriate values
- is_imported_content is set to 'true'
All fields are present and non-empty, following the documentation requirements.
pages/notices/pectra-changes.mdx (1)
2-15: Title case adjusted to sentence case correctly.The page title has been properly updated to use sentence case for "breaking changes" instead of title case, which aligns with the style guide requirements for headers (H1, H2, H3).
All required metadata fields are present and properly formatted:
- title (with sentence case for "breaking changes")
- description
- lang
- content_type
- topic
- personas (with appropriate roles for this technical content)
- categories (with relevant technical categories)
- is_imported_content set to 'false'
The metadata structure meets documentation requirements.
pages/connect/contribute/docs-contribute.mdx (1)
2-15: Title simplified and metadata structure complete.The title has been simplified from "Ways to contribute to Optimism Docs" to "Contribute to Optimism Docs," making it more concise while retaining its meaning.
All required metadata fields are present and non-empty:
- title (simplified appropriately)
- description (clear and concise)
- lang
- content_type
- topic
- personas (with all four relevant developer roles)
- categories (with appropriate values)
- is_imported_content set to 'false'
The metadata structure follows documentation requirements.
pages/connect/contribute/style-guide.mdx (1)
2-17: Title simplified and metadata structure complete.The title has been simplified from "Optimism Docs style guide" to "Docs style guide" which is more concise. The description is now formatted using YAML's multi-line string format (using >-), which improves readability in the frontmatter.
All required metadata fields are present and properly formatted:
- title (simplified appropriately)
- description (using proper YAML multi-line format)
- lang
- content_type
- topic
- personas (with all four developer roles)
- categories (with appropriate values)
- is_imported_content set to 'false'
The metadata structure follows documentation requirements.
utils/metadata-manager.ts (9)
39-39: Simplified error message for cleaner validation output.Error message has been simplified from a full descriptive message to just the field name. This is consistent with other similar changes in this file and provides more concise error reporting.
45-45: Simplified error message for cleaner validation output.Error message has been simplified from a full descriptive message to just the field name. This is consistent with other similar changes in this file and provides more concise error reporting.
47-47: Error message now includes minimum value requirement.The error message has been simplified but still retains the important minimum value information, improving clarity while keeping the message concise.
53-53: Simplified error message for cleaner validation output.Error message has been simplified from a full descriptive message to just the field name. This is consistent with other similar changes in this file and provides more concise error reporting.
59-59: Simplified error message for cleaner validation output.Error message has been simplified from a full descriptive message to just the field name. This is consistent with other similar changes in this file and provides more concise error reporting.
61-61: Error message now includes minimum value requirement.The error message has been simplified but still retains the important minimum value information, improving clarity while keeping the message concise.
70-70: Improved error message specificity.Error message has been refined to clearly indicate it's an "invalid persona" rather than a generic validation error, making it easier to understand the specific validation failure.
78-78: Improved error message specificity.Error message has been refined to clearly indicate it's an "invalid content_type" rather than a generic validation error, making it easier to understand the specific validation failure.
86-86: Improved error message specificity.Error message has been refined to clearly indicate it's an "invalid category" rather than a generic validation error, making it easier to understand the specific validation failure.
utils/metadata-batch-cli.ts (6)
9-9: Import the constants nicely doneGood addition of importing VALID_CATEGORIES and VALID_PERSONAS constants, which makes the code cleaner and more maintainable.
174-175: Good logging enhancementAdding this log statement provides better visibility into the processing flow and gives users confirmation of how many files will be processed.
189-190: Improved error logging with actionable suggestionsThe enhanced error output with suggested metadata values makes it much easier for users to fix issues. Using the detected categories from the analyzer instead of hardcoding values is a good practice.
Also applies to: 194-200
203-206: Clear success indicatorsImproved messaging for both normal runs and dry runs helps clarify what actions were taken.
215-218: Simplified summary outputMore concise summary output makes it easier to understand the results at a glance.
229-236: Refactored file selection logicBetter organization of the file selection logic with clear comments explaining the two modes (direct command vs PR validation).
pages/connect/resources/glossary.mdx (1)
1-19: Metadata is properly updatedAll required metadata fields for a regular content page are present and correctly formatted:
- title
- description
- lang
- content_type
- topic
- personas (with appropriate values)
- categories
- is_imported_content
utils/metadata-analyzer.ts (2)
405-437: Simplified category detection logicThe category detection logic has been streamlined to focus on specific checks that appear to be most relevant. This makes the function more maintainable.
551-568: Good structured suggestions implementationThe updated
analyzeContentfunction now returns empty required fields for validation but includes detailed suggestions in a separatesuggestionsobject. This is a good pattern that allows for more flexible validation handling.
| } | ||
|
|
||
| export async function updateMetadataFile( | ||
| filepath: string, | ||
| options: { | ||
| dryRun?: boolean; | ||
| verbose?: boolean; | ||
| analysis: MetadataResult; | ||
| validateOnly: boolean; | ||
| prMode: boolean; | ||
| } | ||
| ): Promise<{ isValid: boolean; errors: string[]; metadata: MetadataResult }> { | ||
| try { | ||
| const content = await fs.readFile(filepath, 'utf8') | ||
| const { data: frontmatter } = matter(content) | ||
| const result = await validateMetadata(options.analysis, filepath, options) | ||
|
|
||
| // Return early if validation failed | ||
| if (!result.isValid) { | ||
| return { | ||
| isValid: false, | ||
| errors: result.errors, | ||
| metadata: options.analysis | ||
| } | ||
| } | ||
|
|
||
| // ... rest of function | ||
| } catch (error) { | ||
| throw new Error(`Failed to update metadata for ${filepath}: ${error.message}`) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation of new updateMetadataFile function is incomplete.
The new function for updating metadata files is missing its implementation details. The function starts correctly by reading the file and validating the metadata, but it's incomplete with "... rest of function" commented. If validation fails, it returns early with the appropriate error information, but the success path is not implemented.
This function should likely update the file's metadata if validation passes and not in dry-run mode, similar to the updateMetadata function.
Complete the function implementation by adding the code to handle the success path. Consider the following implementation:
export async function updateMetadataFile(
filepath: string,
options: {
dryRun?: boolean;
verbose?: boolean;
analysis: MetadataResult;
validateOnly: boolean;
prMode: boolean;
}
): Promise<{ isValid: boolean; errors: string[]; metadata: MetadataResult }> {
try {
const content = await fs.readFile(filepath, 'utf8')
const { data: frontmatter } = matter(content)
const result = await validateMetadata(options.analysis, filepath, options)
// Return early if validation failed
if (!result.isValid) {
return {
isValid: false,
errors: result.errors,
metadata: options.analysis
}
}
- // ... rest of function
+ // If validate only, just return success
+ if (options.validateOnly) {
+ return {
+ isValid: true,
+ errors: [],
+ metadata: options.analysis
+ }
+ }
+
+ // If not dry run, update the file
+ if (!options.dryRun) {
+ const { content: docContent } = matter(content)
+ const updatedContent = matter.stringify(docContent, options.analysis)
+ await fs.writeFile(filepath, updatedContent, 'utf8')
+ }
+
+ // Return success
+ return {
+ isValid: true,
+ errors: [],
+ metadata: options.analysis
+ }
} catch (error) {
throw new Error(`Failed to update metadata for ${filepath}: ${error.message}`)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } | |
| export async function updateMetadataFile( | |
| filepath: string, | |
| options: { | |
| dryRun?: boolean; | |
| verbose?: boolean; | |
| analysis: MetadataResult; | |
| validateOnly: boolean; | |
| prMode: boolean; | |
| } | |
| ): Promise<{ isValid: boolean; errors: string[]; metadata: MetadataResult }> { | |
| try { | |
| const content = await fs.readFile(filepath, 'utf8') | |
| const { data: frontmatter } = matter(content) | |
| const result = await validateMetadata(options.analysis, filepath, options) | |
| // Return early if validation failed | |
| if (!result.isValid) { | |
| return { | |
| isValid: false, | |
| errors: result.errors, | |
| metadata: options.analysis | |
| } | |
| } | |
| // ... rest of function | |
| } catch (error) { | |
| throw new Error(`Failed to update metadata for ${filepath}: ${error.message}`) | |
| } | |
| } | |
| } | |
| export async function updateMetadataFile( | |
| filepath: string, | |
| options: { | |
| dryRun?: boolean; | |
| verbose?: boolean; | |
| analysis: MetadataResult; | |
| validateOnly: boolean; | |
| prMode: boolean; | |
| } | |
| ): Promise<{ isValid: boolean; errors: string[]; metadata: MetadataResult }> { | |
| try { | |
| const content = await fs.readFile(filepath, 'utf8') | |
| const { data: frontmatter } = matter(content) | |
| const result = await validateMetadata(options.analysis, filepath, options) | |
| // Return early if validation failed | |
| if (!result.isValid) { | |
| return { | |
| isValid: false, | |
| errors: result.errors, | |
| metadata: options.analysis | |
| } | |
| } | |
| // If validate only, just return success | |
| if (options.validateOnly) { | |
| return { | |
| isValid: true, | |
| errors: [], | |
| metadata: options.analysis | |
| } | |
| } | |
| // If not dry run, update the file | |
| if (!options.dryRun) { | |
| const { content: docContent } = matter(content) | |
| const updatedContent = matter.stringify(docContent, options.analysis) | |
| await fs.writeFile(filepath, updatedContent, 'utf8') | |
| } | |
| // Return success | |
| return { | |
| isValid: true, | |
| errors: [], | |
| metadata: options.analysis | |
| } | |
| } catch (error) { | |
| throw new Error(`Failed to update metadata for ${filepath}: ${error.message}`) | |
| } | |
| } |
| const stats = { | ||
| total: mdxFiles.length, | ||
| successful: 0, | ||
| needsReview: 0, | ||
| failed: 0 | ||
| } | ||
|
|
||
| console.log(`Found ${mdxFiles.length} valid files to check\n`) | ||
|
|
||
| let processedCount = 0 | ||
| let needsReviewCount = 0 | ||
|
|
||
| for (const file of mdxFiles) { | ||
| try { | ||
| const metadata = await generateMetadata(file) | ||
| const content = await fs.readFile(file, 'utf8') | ||
| const { data: frontmatter } = matter(content) | ||
| const analysis = analyzeContent(content, file, isVerbose) | ||
| const result = await updateMetadataFile(file, { | ||
| dryRun: isDryRun, | ||
| verbose: isVerbose, | ||
| analysis, | ||
| validateOnly: false, | ||
| prMode: false, | ||
| analysis: metadata | ||
| prMode: false | ||
| }) | ||
|
|
||
| processedCount++ | ||
|
|
||
| // Show metadata for each file | ||
| console.log(`\nFile: ${file}`) | ||
| console.log('Categories:', metadata.categories?.join(', ') || 'none') | ||
|
|
||
| console.log(`File: ${file}`) | ||
|
|
||
| if (!result.isValid) { | ||
| needsReviewCount++ | ||
| console.log('\x1b[33m⚠️ Review needed:\x1b[0m') | ||
| result.errors.forEach(error => console.log(` → ${error}`)) | ||
| stats.needsReview++ | ||
| const filename = file.split('/').pop()?.replace('.mdx', '') | ||
|
|
||
| // Use the analyzer's detected categories instead of hardcoding | ||
| const suggestedCategories = analysis.suggestions?.categories || ['protocol'] | ||
|
|
||
| console.log(`${colors.yellow}⚠️ Missing: ${result.errors.join(', ')}${colors.reset}`) | ||
| console.log(`Suggested: content_type: guide, topic: ${filename}, personas: [${VALID_PERSONAS[0]}], categories: ${JSON.stringify(suggestedCategories)}\n`) | ||
| } else { | ||
| if (!isDryRun) { | ||
| console.log(' ✓ Updates applied\n') | ||
| } else { | ||
| console.log(' ✓ Validation passed (dry run)\n') | ||
| } | ||
| stats.successful++ | ||
| } | ||
| } catch (error) { | ||
| stats.failed++ | ||
| console.error(`Error processing ${file}:`, error) | ||
| } | ||
| } | ||
|
|
||
| // Summary with colors | ||
| console.log(`\n${processedCount} files processed`) | ||
| if (needsReviewCount === 0) { | ||
| console.log('\x1b[32m✓ All files have valid metadata\x1b[0m') | ||
| } else { | ||
| console.log(`\x1b[33m⚠️ ${needsReviewCount} files need review\x1b[0m`) | ||
| console.log(`${stats.total} files processed`) | ||
| if (stats.needsReview > 0) { | ||
| console.log(`${colors.yellow}⚠️ ${stats.needsReview} files need review${colors.reset}`) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Eliminate code duplication
There appears to be significant code duplication between the processFiles function and the main function. Both contain nearly identical file processing logic, stats tracking, and output formatting.
Consider refactoring to eliminate this duplication by having the main function call the processFiles function instead of reimplementing the same logic:
async function main() {
try {
const isDryRun = process.argv.includes('--dry-run')
const isVerbose = process.argv.includes('--verbose')
// Get files from either command line patterns or CHANGED_FILES
let mdxFiles = []
const patterns = process.argv.slice(2).filter(arg => !arg.startsWith('--'))
if (patterns.length > 0) {
// Direct command: use provided patterns
mdxFiles = await globby(patterns)
} else if (process.env.CHANGED_FILES) {
// PR validation: use changed files
mdxFiles = process.env.CHANGED_FILES.split('\n').filter(Boolean)
}
mdxFiles = mdxFiles.filter(file => file.endsWith('.mdx'))
if (mdxFiles.length === 0) {
console.log('✓ No MDX files to check')
process.exit(0)
}
- const stats = {
- total: mdxFiles.length,
- successful: 0,
- needsReview: 0,
- failed: 0
- }
-
- console.log(`Found ${mdxFiles.length} valid files to check\n`)
-
- for (const file of mdxFiles) {
- try {
- const content = await fs.readFile(file, 'utf8')
- const { data: frontmatter } = matter(content)
- const analysis = analyzeContent(content, file, isVerbose)
- const result = await updateMetadataFile(file, {
- dryRun: isDryRun,
- verbose: isVerbose,
- analysis,
- validateOnly: false,
- prMode: false
- })
-
- console.log(`File: ${file}`)
-
- if (!result.isValid) {
- stats.needsReview++
- const filename = file.split('/').pop()?.replace('.mdx', '')
-
- // Use the analyzer's detected categories instead of hardcoding
- const suggestedCategories = analysis.suggestions?.categories || ['protocol']
-
- console.log(`${colors.yellow}⚠️ Missing: ${result.errors.join(', ')}${colors.reset}`)
- console.log(`Suggested: content_type: guide, topic: ${filename}, personas: [${VALID_PERSONAS[0]}], categories: ${JSON.stringify(suggestedCategories)}\n`)
- } else {
- if (!isDryRun) {
- console.log(' ✓ Updates applied\n')
- } else {
- console.log(' ✓ Validation passed (dry run)\n')
- }
- stats.successful++
- }
- } catch (error) {
- stats.failed++
- console.error(`Error processing ${file}:`, error)
- }
- }
-
- console.log(`${stats.total} files processed`)
- if (stats.needsReview > 0) {
- console.log(`${colors.yellow}⚠️ ${stats.needsReview} files need review${colors.reset}`)
- }
+ // Validate file paths and process them
+ const validFiles = await validateFilePaths(mdxFiles)
+ const result = await processFiles(validFiles, {
+ dryRun: isDryRun,
+ verbose: isVerbose
+ })
+
+ // Exit with error code if there were failures
+ if (result.hasErrors) {
+ process.exit(1)
+ }
} catch (error) {
console.error('\x1b[31mError:\x1b[0m', error)
process.exit(1)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const stats = { | |
| total: mdxFiles.length, | |
| successful: 0, | |
| needsReview: 0, | |
| failed: 0 | |
| } | |
| console.log(`Found ${mdxFiles.length} valid files to check\n`) | |
| let processedCount = 0 | |
| let needsReviewCount = 0 | |
| for (const file of mdxFiles) { | |
| try { | |
| const metadata = await generateMetadata(file) | |
| const content = await fs.readFile(file, 'utf8') | |
| const { data: frontmatter } = matter(content) | |
| const analysis = analyzeContent(content, file, isVerbose) | |
| const result = await updateMetadataFile(file, { | |
| dryRun: isDryRun, | |
| verbose: isVerbose, | |
| analysis, | |
| validateOnly: false, | |
| prMode: false, | |
| analysis: metadata | |
| prMode: false | |
| }) | |
| processedCount++ | |
| // Show metadata for each file | |
| console.log(`\nFile: ${file}`) | |
| console.log('Categories:', metadata.categories?.join(', ') || 'none') | |
| console.log(`File: ${file}`) | |
| if (!result.isValid) { | |
| needsReviewCount++ | |
| console.log('\x1b[33m⚠️ Review needed:\x1b[0m') | |
| result.errors.forEach(error => console.log(` → ${error}`)) | |
| stats.needsReview++ | |
| const filename = file.split('/').pop()?.replace('.mdx', '') | |
| // Use the analyzer's detected categories instead of hardcoding | |
| const suggestedCategories = analysis.suggestions?.categories || ['protocol'] | |
| console.log(`${colors.yellow}⚠️ Missing: ${result.errors.join(', ')}${colors.reset}`) | |
| console.log(`Suggested: content_type: guide, topic: ${filename}, personas: [${VALID_PERSONAS[0]}], categories: ${JSON.stringify(suggestedCategories)}\n`) | |
| } else { | |
| if (!isDryRun) { | |
| console.log(' ✓ Updates applied\n') | |
| } else { | |
| console.log(' ✓ Validation passed (dry run)\n') | |
| } | |
| stats.successful++ | |
| } | |
| } catch (error) { | |
| stats.failed++ | |
| console.error(`Error processing ${file}:`, error) | |
| } | |
| } | |
| // Summary with colors | |
| console.log(`\n${processedCount} files processed`) | |
| if (needsReviewCount === 0) { | |
| console.log('\x1b[32m✓ All files have valid metadata\x1b[0m') | |
| } else { | |
| console.log(`\x1b[33m⚠️ ${needsReviewCount} files need review\x1b[0m`) | |
| console.log(`${stats.total} files processed`) | |
| if (stats.needsReview > 0) { | |
| console.log(`${colors.yellow}⚠️ ${stats.needsReview} files need review${colors.reset}`) | |
| } | |
| async function main() { | |
| try { | |
| const isDryRun = process.argv.includes('--dry-run') | |
| const isVerbose = process.argv.includes('--verbose') | |
| // Get files from either command line patterns or CHANGED_FILES | |
| let mdxFiles = [] | |
| const patterns = process.argv.slice(2).filter(arg => !arg.startsWith('--')) | |
| if (patterns.length > 0) { | |
| // Direct command: use provided patterns | |
| mdxFiles = await globby(patterns) | |
| } else if (process.env.CHANGED_FILES) { | |
| // PR validation: use changed files | |
| mdxFiles = process.env.CHANGED_FILES.split('\n').filter(Boolean) | |
| } | |
| mdxFiles = mdxFiles.filter(file => file.endsWith('.mdx')) | |
| if (mdxFiles.length === 0) { | |
| console.log('✓ No MDX files to check') | |
| process.exit(0) | |
| } | |
| // Validate file paths and process them | |
| const validFiles = await validateFilePaths(mdxFiles) | |
| const result = await processFiles(validFiles, { | |
| dryRun: isDryRun, | |
| verbose: isVerbose | |
| }) | |
| // Exit with error code if there were failures | |
| if (result.hasErrors) { | |
| process.exit(1) | |
| } | |
| } catch (error) { | |
| console.error('\x1b[31mError:\x1b[0m', error) | |
| process.exit(1) | |
| } | |
| } |