-
Notifications
You must be signed in to change notification settings - Fork 262
Add Husky pre-commit and pre-push hooks for automated doc checks #1413
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
Conversation
✅ Deploy Preview for docs-optimism ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Warning Rate limit exceeded@krofax has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 33 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request introduces a new Git pre-push hook by adding a Sequence Diagram(s)sequenceDiagram
participant Developer
participant Git
participant PrePushHook
Developer->>Git: Initiate push
Git->>PrePushHook: Execute .husky/pre-push hook
PrePushHook->>PrePushHook: Source Husky environment
PrePushHook->>Output: Print "automatic fixes are being applied" message
PrePushHook->>PNPM: Execute "pnpm fix"
PNPM-->>PrePushHook: Fix process complete
PrePushHook->>Output: Print confirmation message
PrePushHook->>Git: Continue push operation
Possibly related PRs
Suggested reviewers
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: 0
🧹 Nitpick comments (2)
.husky/pre-push (1)
1-4
: Add error handling and user feedback.While the commands are correctly sequenced, consider enhancing the script with error handling and user feedback:
. "$(dirname -- "$0")/_/husky.sh" -pnpm fix-redirects -pnpm link-checker +echo "Running pre-push checks..." + +echo "📝 Fixing redirects..." +if ! pnpm fix-redirects; then + echo "❌ Fix redirects failed" + exit 1 +fi + +echo "🔍 Checking links..." +if ! pnpm link-checker; then + echo "❌ Link checker failed" + exit 1 +fi + +echo "✅ All pre-push checks passed"package.json (1)
22-31
: Consider optimizing lint-staged configuration.The current configuration runs fix-redirects and link-checker on all files, which might be inefficient for large repositories.
"lint-staged": { "**/*.mdx": [ "eslint --max-warnings 0", "cspell lint" ], - "**/*": [ + "**/*.{md,mdx}": [ "pnpm fix-redirects", "pnpm link-checker" ] }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
.husky/pre-commit
(1 hunks).husky/pre-push
(1 hunks)package.json
(3 hunks)pages/app-developers/tutorials/transactions/sdk-estimate-costs.mdx
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- pages/app-developers/tutorials/transactions/sdk-estimate-costs.mdx
- .husky/pre-commit
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules - docs-optimism
- GitHub Check: Header rules - docs-optimism
- GitHub Check: Pages changed - docs-optimism
- GitHub Check: pr-workflow
🔇 Additional comments (1)
package.json (1)
65-66
: LGTM! Dependencies are up to date.The latest versions of husky (^9.1.7) and lint-staged (^15.4.3) are being used.
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: 0
🧹 Nitpick comments (1)
package.json (1)
19-20
: Suppress Warning Messages in Metadata Validation Scripts
Thevalidate-metadata
andvalidate-pr-metadata
scripts now setNODE_OPTIONS="--no-warnings"
to suppress warning messages during execution. This reduces noise in logs but please verify that important warnings are not being unintentionally hidden in development or CI environments.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json
(3 hunks)utils/metadata-analyzer.ts
(1 hunks)utils/metadata-manager.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- utils/metadata-manager.ts
- utils/metadata-analyzer.ts
⏰ 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 (3)
package.json (3)
24-25
: Enable Husky Hook Installation and Optimize Postbuild Tasks
The addition of the"prepare": "husky"
script ensures that Husky hooks are automatically set up during the preparation phase, which aligns with the PR’s goal of automating quality checks. The new"postbuild": "next-sitemap"
script should correctly generate the sitemap after building. Confirm that these changes integrate seamlessly with your build and deployment pipelines.
27-35
: Establish Lint-Staged Configuration for Automated Doc Checks
The newly added"lint-staged"
section configures targeted commands for staged files:
- For
**/*.mdx
files: running ESLint (with a no-warning threshold) and a spell check viacspell lint
- For all files: running
pnpm fix-redirects
andpnpm link-checker
This setup directly supports the PR objective of enforcing documentation quality before commits and pushes. Ensure that the glob patterns precisely match the intended files to avoid unintentional processing.
72-73
: Add DevDependencies for Husky and Lint-Staged
The new devDependencies"husky": "^9.1.7"
and"lint-staged": "^15.4.3"
are included to support the hook and staged file validation workflows. Validate that these versions are fully compatible with your project requirements and any related tooling.
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: 3
🧹 Nitpick comments (2)
.husky/pre-push (2)
1-6
: Add a missing final newline to the file.The file should end with a newline character to follow best practices for text files.
echo "✅ Fixes applied - your contribution is now ready for review!" +
4-6
: Consider adding more detailed output about which fixes are being applied.To improve user experience, it would be helpful to provide more information about what specifically is being fixed.
echo "🔍 Running automatic fixes before pushing..." -pnpm fix +echo " - Checking and fixing documentation issues..." +echo " - Running spell checker..." +echo " - Validating links..." +echo " - Generating breadcrumbs..." +echo " - Fixing redirects..." +pnpm fix echo "✅ Fixes applied - your contribution is now ready for review!"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.husky/pre-push
(1 hunks)package.json
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
⏰ 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 (1)
.husky/pre-push (1)
1-2
: The shebang and Husky initialization look good.The script correctly initializes the Husky environment which is essential for Git hooks to work properly.
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: 0
🧹 Nitpick comments (3)
utils/create-breadcrumbs.ts (3)
5-5
: Path resolution change improves script execution in different contexts.The change from using
__dirname
toprocess.cwd()
is a good improvement that makes the script more robust when executed from different contexts, particularly when run as part of Git hooks like the Husky pre-commit and pre-push hooks introduced in this PR. This ensures the script can correctly locate thepages
directory regardless of the execution context.Consider adding a check to verify that the pages directory exists and provide a meaningful error message if it doesn't:
const rootDir = path.join(process.cwd(), 'pages'); + +// Verify that the pages directory exists +try { + fs.access(rootDir).catch(() => { + console.error(`Error: Pages directory not found at ${rootDir}`); + console.error('Make sure the script is being run from the project root'); + process.exit(1); + }); +} catch (error) { + // fs.access is asynchronous, handle any synchronous errors here + console.error(`Error checking pages directory: ${error.message}`); + process.exit(1); +}
63-82
: Function logic is sound but could benefit from error handling enhancement.The
generateFolderDescription
function effectively creates descriptive content for folders based on their contents, with good fallback mechanisms. However, there's no error handling if the content extraction fails unexpectedly.Consider adding a try-catch block to handle potential errors during content extraction:
async function generateFolderDescription(folderName: string, files: FileInfo[]): Promise<string> { if (files.length === 0) { return `Documentation for ${toTitleCase(folderName)} is coming soon.`; } // Try to find overview or introduction file const overviewFile = files.find(file => file.url.toLowerCase().includes('overview') || file.url.toLowerCase().includes('introduction') ); if (overviewFile && overviewFile.content) { + try { const desc = extractFirstParagraph(overviewFile.content); if (desc) return desc; + } catch (error) { + console.warn(`Warning: Failed to extract paragraph from ${overviewFile.url}: ${error.message}`); + // Continue to fallback + } } // Generate description from files if no overview is found const topics = files.map(file => toTitleCase(path.basename(file.url))).join(', '); return `Documentation covering ${topics} in the ${toTitleCase(folderName)} section of the OP Stack ecosystem.`; }🧰 Tools
🪛 Biome (1.9.4)
[error] 74-74: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
232-254
: Debug logging enhancement needed for better troubleshooting.The main function logs basic information about the process but could benefit from more detailed logging, especially when errors occur in the nested functions.
Enhance the error reporting in the main function to provide more context:
const main = async (): Promise<void> => { console.log('Starting breadcrumb update process...'); console.log('Root directory:', rootDir); try { // Process main sections: builders, chain, connect, stack const mainSections = ['app-developers', 'operators', 'superchain', 'stack']; for (const section of mainSections) { const sectionPath = path.join(rootDir, section); try { await fs.access(sectionPath); await processSubfolders(sectionPath); console.log(`Completed processing ${section} section`); } catch (error) { - console.log(`Skipping ${section} - directory not found`); + console.log(`Skipping ${section} - ${error.code === 'ENOENT' ? 'directory not found' : `error: ${error.message}`}`); } } console.log('Finished updating all breadcrumbs.'); } catch (error) { - console.error('Error in main process:', error); + console.error(`Error in main process: ${error.message}`); + console.error(error.stack); process.exit(1); } };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
package.json
(3 hunks)utils/create-breadcrumbs.ts
(1 hunks)utils/fix-redirects.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- utils/fix-redirects.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🔇 Additional comments (1)
utils/create-breadcrumbs.ts (1)
238-238
:❓ Verification inconclusive
Verify the main sections array against the current project structure.
The main sections array hardcodes specific folder names (
app-developers
,operators
,superchain
,stack
), which might need maintenance as the project evolves.
🏁 Script executed:
#!/bin/bash # Description: Verify that the main sections listed in the script match the actual directory structure echo "Checking if the main sections exist in the pages directory..." # Get the immediate directories in the pages directory DIRS=$(find pages -maxdepth 1 -type d | grep -v "^pages$" | sed 's|pages/||') echo "Found directories: $DIRS" # Check if each main section exists for section in 'app-developers' 'operators' 'superchain' 'stack'; do if echo "$DIRS" | grep -q "$section"; then echo "✅ Section '$section' exists" else echo "❌ Section '$section' not found in pages directory" fi done # Check if there are additional directories not included in the script for dir in $DIRS; do if ! echo "app-developers operators superchain stack" | grep -q "$dir"; then echo "⚠️ Directory '$dir' exists but is not included in the script's main sections" fi doneLength of output: 2607
Main Sections Array Consistency Check
The hardcoded array (
['app-developers', 'operators', 'superchain', 'stack']
) correctly includes the primary folders since they exist as expected in thepages
directory. However, the directory scan also revealed additional folders—specifically notices, get-started, and connect—that are not present in the array. Please review whether these extra directories should be added to the main sections array or if they are meant to be managed separately.
Description
Overview
This PR implements Git hooks using Husky to automate documentation quality checks, ensuring all contributions maintain consistent quality standards before they're committed or pushed.
Changes
Benefits
Tests
Additional context
Metadata