Automating the doc schema generation using husky. #2851
Conversation
WalkthroughThe pull request introduces modifications to the pre-commit hook script, enhancing the development workflow by removing the command for generating the schema and adjusting the sequence of commands executed during the commit process. A new command Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
package.json (1)
26-27: Consider removing outdated scriptsgenerate:graphql-markdownandgenerate:graphql-schemaWith the introduction of the new
generate:docsscript, the previous scriptsgenerate:graphql-markdownandgenerate:graphql-schemamay no longer be necessary. Removing them can reduce confusion and redundancy.Apply this diff to remove the outdated scripts:
- "generate:graphql-markdown": "graphql-markdown http://localhost:4000/graphql > docs/docs/Schema.md", - "generate:graphql-schema": "get-graphql-schema http://localhost:4000/graphql --json > docs/github-actions/schema.json",🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Package version mismatch: lock file's @types/[email protected] does not satisfy @types/[email protected]
docs/docs/Schema.md (2)
3-239: Enhance documentation structure for better readabilityConsider organizing the table of contents hierarchically by grouping related types (e.g., Event-related types, User-related types, etc.) instead of a flat alphabetical list. This would make it easier for developers to find related functionality.
Example structure:
## Table of Contents ### Core Types - Organization - User - Group ### Event Management - Event - EventAttendee - EventVolunteer - EventVolunteerGroup ### Content Management - Post - Comment - Advertisement ...
2630-2641: Enhance connection types with additional fieldsThe
UserTagsConnectiontype could benefit from additional fields to support more advanced pagination and filtering scenarios.Consider adding:
type UserTagsConnection { edges: [UserTagsConnectionEdge!]! pageInfo: DefaultConnectionPageInfo! totalCount: Int # Add these fields filteredCount: Int hasMore: Boolean filterInfo: FilterMetadata } type FilterMetadata { appliedFilters: [String!] availableFilters: [String!] }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
.husky/pre-commit(1 hunks)docs/docs/Schema.md(1 hunks)package.json(3 hunks)scripts/schema-generator.mjs(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: PR Workflow
.husky/pre-commit
[error] Unauthorized file modification or deletion attempt
scripts/schema-generator.mjs
[error] Unauthorized file modification or deletion attempt
package.json
[error] Package version mismatch: lock file's @types/[email protected] does not satisfy @types/[email protected]
🪛 eslint
scripts/schema-generator.mjs
[error] 30-30: 'schemaStr' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🪛 Markdownlint (0.37.0)
docs/docs/Schema.md
645-645: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style
(MD055, table-pipe-style)
646-646: Expected: leading_and_trailing; Actual: trailing_only; Missing leading pipe
Table pipe style
(MD055, table-pipe-style)
647-647: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style
(MD055, table-pipe-style)
648-648: Expected: leading_and_trailing; Actual: trailing_only; Missing leading pipe
Table pipe style
(MD055, table-pipe-style)
649-649: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style
(MD055, table-pipe-style)
650-650: Expected: leading_and_trailing; Actual: trailing_only; Missing leading pipe
Table pipe style
(MD055, table-pipe-style)
651-651: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style
(MD055, table-pipe-style)
652-652: Expected: leading_and_trailing; Actual: trailing_only; Missing leading pipe
Table pipe style
(MD055, table-pipe-style)
2570-2570: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style
(MD055, table-pipe-style)
2571-2571: Expected: leading_and_trailing; Actual: trailing_only; Missing leading pipe
Table pipe style
(MD055, table-pipe-style)
2575-2575: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style
(MD055, table-pipe-style)
2576-2576: Expected: leading_and_trailing; Actual: trailing_only; Missing leading pipe
Table pipe style
(MD055, table-pipe-style)
2577-2577: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style
(MD055, table-pipe-style)
2578-2578: Expected: leading_and_trailing; Actual: trailing_only; Missing leading pipe
Table pipe style
(MD055, table-pipe-style)
646-646: Expected: 3; Actual: 1; Too few cells, row will be missing data
Table column count
(MD056, table-column-count)
648-648: Expected: 3; Actual: 1; Too few cells, row will be missing data
Table column count
(MD056, table-column-count)
650-650: Expected: 3; Actual: 1; Too few cells, row will be missing data
Table column count
(MD056, table-column-count)
652-652: Expected: 3; Actual: 1; Too few cells, row will be missing data
Table column count
(MD056, table-column-count)
2571-2571: Expected: 3; Actual: 1; Too few cells, row will be missing data
Table column count
(MD056, table-column-count)
2576-2576: Expected: 3; Actual: 1; Too few cells, row will be missing data
Table column count
(MD056, table-column-count)
2578-2578: Expected: 3; Actual: 1; Too few cells, row will be missing data
Table column count
(MD056, table-column-count)
🪛 LanguageTool
docs/docs/Schema.md
[typographical] ~1322-~1322: If you want to indicate numerical ranges or time ranges, consider using an en dash.
Context: ...epresent values between -(2^31) and 2^31 - 1. ### InvalidCursor #### Fields | Fi...
(DASH_RULE)
[style] ~2843-~2843: Using many exclamation marks might seem excessive (in this case: 639 exclamation marks for a text that’s 57918 characters long)
Context: ...| | title | String! | | | userIds | [ID!]! | |
(EN_EXCESSIVE_EXCLAMATION)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (2)
scripts/schema-generator.mjs (1)
78-79: Verify if writing to 'docs/github-actions/' and 'docs/docs/' is allowedThe script writes files to
docs/github-actions/schema.jsonanddocs/docs/Schema.md. According to the pipeline failure:[error] Unauthorized file modification or deletion attemptPlease verify whether modifying files in these directories is permitted in the pre-commit hook or CI pipeline.
Run the following script to check if modifications to these directories are causing the pipeline failure:
Also applies to: 95-97, 101-104
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized file modification or deletion attempt
package.json (1)
118-118: Verify if@graphql-markdown/docusaurusis requiredThe dependency
@graphql-markdown/docusaurushas been added, but it appears that the newschema-generator.mjsscript does not utilize it. Please verify if this dependency is necessary, and remove it if it's unused to keep the dependencies lean.🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Package version mismatch: lock file's @types/[email protected] does not satisfy @types/[email protected]
scripts/schema-generator.mjs
Outdated
| } | ||
|
|
||
| function generateSimpleMarkdown(schema) { | ||
| const schemaStr = printSchema(schema); |
There was a problem hiding this comment.
Remove unused variable schemaStr
The variable schemaStr is assigned a value but never used, leading to a linting error:
[error] 30-30: 'schemaStr' is assigned a value but never used. (@typescript-eslint/no-unused-vars)
Please remove it to clean up the code.
Apply this diff to fix the issue:
- const schemaStr = printSchema(schema);🧰 Tools
🪛 eslint
[error] 30-30: 'schemaStr' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Actions: PR Workflow
[error] Unauthorized file modification or deletion attempt
.husky/pre-commit
Outdated
| @@ -13,6 +13,7 @@ npm run typecheck | |||
| npm run lint-staged | |||
|
|
|||
| npm run gen:schema | |||
| npm run generate:docs | |||
There was a problem hiding this comment.
💡 Codebase verification
Remove generate:docs from pre-commit hook to prevent pipeline failures
The generate:docs script modifies git-tracked files in the docs/ directory, which can cause the pipeline to fail with "Unauthorized file modification" errors. Documentation generation should be moved to the CI/CD pipeline instead of running during pre-commit to:
- Prevent commit failures due to unauthorized file modifications
- Ensure consistent documentation across all branches
- Avoid race conditions during concurrent commits
🔗 Analysis chain
Verify the impact of running npm run generate:docs in pre-commit
Adding npm run generate:docs to the pre-commit hook may cause the script to modify files that are not supposed to be changed during commit, leading to the pipeline failure:
[error] Unauthorized file modification or deletion attempt
Please verify whether running this command in the pre-commit hook is appropriate and allowed.
Run the following script to identify files modified by the pre-commit hook:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check files modified by the pre-commit hook.
# Temporarily stash any staged changes
git stash -k --keep-index
# Run the pre-commit script
.husky/pre-commit
# List files modified by the pre-commit hook
git diff --name-only
# Restore the stashed changes
git stash pop
Length of output: 135
Script:
#!/bin/bash
# Check package.json for generate:docs script and search for documentation patterns
echo "=== Package.json scripts ==="
cat package.json | jq -r '.scripts | select(.["generate:docs"] != null)'
echo -e "\n=== Documentation output patterns ==="
# Look for common documentation output directories
fd -t d '(docs?|documentation|api-docs|reference)$'
echo -e "\n=== Documentation generation patterns ==="
# Search for documentation generation patterns
rg -l -i '(generate|build|create).*docs?' --type js --type json --type ts
Length of output: 3446
Script:
#!/bin/bash
# Examine schema generator implementation
echo "=== Schema Generator Content ==="
cat scripts/schema-generator.mjs
echo -e "\n=== Files in docs directory ==="
# Check if docs files are tracked
git ls-files docs/
echo -e "\n=== Git ignore patterns ==="
# Check if docs are ignored
cat .gitignore | grep -i "doc"
Length of output: 4268
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized file modification or deletion attempt
| | Field | Type | Description | | ||
| |-------|------|-------------| | ||
| | success | Boolean! | | |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding validation constraints to input types
Input types like UpdateUserInput lack validation constraints. Consider adding:
- Length constraints for string fields
- Range constraints for numeric fields
- Format validation for specialized fields
Example:
input UpdateUserInput {
firstName: String! @constraint(minLength: 1, maxLength: 50)
lastName: String! @constraint(minLength: 1, maxLength: 50)
email: EmailAddress! @constraint(format: "email")
phone: UserPhoneInput
birthDate: Date @constraint(min: "1900-01-01")
}Also applies to: 2372-2388
| | createDonation | Donation! | | | ||
| | createEvent | Event! | | | ||
| | createEventVolunteer | EventVolunteer! | | | ||
| | createEventVolunteerGroup | EventVolunteerGroup! | | | ||
| | createFund | Fund! | | | ||
| | createFundraisingCampaign | FundraisingCampaign! | | | ||
| | createFundraisingCampaignPledge | FundraisingCampaignPledge! | | | ||
| | createMember | CreateMemberPayload! | | | ||
| | createNote | Note! | | | ||
| | createOrganization | Organization! | | | ||
| | createPlugin | Plugin! | | | ||
| | createPost | Post | | | ||
| | createSampleOrganization | Boolean! | | | ||
| | createUserFamily | UserFamily! | | | ||
| | createUserTag | UserTag | | | ||
| | createVenue | Venue | | | ||
| | createVolunteerMembership | VolunteerMembership! | | | ||
| | deleteAdvertisement | DeleteAdvertisementPayload | | | ||
| | deleteAgendaCategory | ID! | | | ||
| | deleteDonationById | DeletePayload! | | | ||
| | deleteNote | ID! | | | ||
| | deleteVenue | Venue | | | ||
| | editVenue | Venue | | | ||
| | forgotPassword | Boolean! | | | ||
| | inviteEventAttendee | EventAttendee! | | | ||
| | joinPublicOrganization | User! | | | ||
| | leaveOrganization | User! | | | ||
| | likeComment | Comment | | | ||
| | likePost | Post | | | ||
| | login | AuthData! | | | ||
| | logout | Boolean! | | | ||
| | markChatMessagesAsRead | Chat! | | | ||
| | otp | OtpData! | | | ||
| | recaptcha | Boolean! | | | ||
| | refreshToken | ExtendSession! | | | ||
| | registerEventAttendee | EventAttendee! | | | ||
| | registerForEvent | EventAttendee! | | | ||
| | rejectMembershipRequest | MembershipRequest! | | | ||
| | removeActionItem | ActionItem! | | | ||
| | removeAdmin | AppUserProfile! | | | ||
| | removeAdvertisement | Advertisement | | | ||
| | removeAgendaItem | AgendaItem! | | | ||
| | removeAgendaSection | ID! | | | ||
| | removeComment | Comment | | | ||
| | removeEvent | Event! | | | ||
| | removeEventAttendee | User! | | | ||
| | removeEventVolunteer | EventVolunteer! | | | ||
| | removeEventVolunteerGroup | EventVolunteerGroup! | | | ||
| | removeFromUserTags | UserTag | | | ||
| | removeFundraisingCampaignPledge | FundraisingCampaignPledge! | | | ||
| | removeMember | Organization! | | | ||
| | removeOrganization | UserData! | | | ||
| | removeOrganizationCustomField | OrganizationCustomField! | | | ||
| | removeOrganizationImage | Organization! | | | ||
| | removePost | Post | | | ||
| | removeSampleOrganization | Boolean! | | | ||
| | removeUserCustomData | UserCustomData! | | | ||
| | removeUserFamily | UserFamily! | | | ||
| | removeUserFromUserFamily | UserFamily! | | | ||
| | removeUserImage | User! | | | ||
| | removeUserTag | UserTag | | | ||
| | resetCommunity | Boolean! | | | ||
| | revokeRefreshTokenForUser | Boolean! | | | ||
| | saveFcmToken | Boolean! | | | ||
| | sendMembershipRequest | MembershipRequest! | | | ||
| | sendMessageToChat | ChatMessage! | | | ||
| | signUp | AuthData! | | | ||
| | togglePostPin | Post! | | | ||
| | unassignUserTag | User | | | ||
| | unblockUser | User! | | | ||
| | unlikeComment | Comment | | | ||
| | unlikePost | Post | | | ||
| | unregisterForEventByUser | Event! | | | ||
| | updateActionItem | ActionItem | | | ||
| | updateActionItemCategory | ActionItemCategory | | | ||
| | updateAdvertisement | UpdateAdvertisementPayload | | | ||
| | updateAgendaCategory | AgendaCategory | | | ||
| | updateAgendaItem | AgendaItem | | | ||
| | updateAgendaSection | AgendaSection | | | ||
| | updateChat | Chat! | | | ||
| | updateChatMessage | ChatMessage! | | | ||
| | updateCommunity | Boolean! | | | ||
| | updateEvent | Event! | | | ||
| | updateEventVolunteer | EventVolunteer! | | | ||
| | updateEventVolunteerGroup | EventVolunteerGroup! | | | ||
| | updateFund | Fund! | | | ||
| | updateFundraisingCampaign | FundraisingCampaign! | | | ||
| | updateFundraisingCampaignPledge | FundraisingCampaignPledge! | | | ||
| | updateLanguage | User! | | | ||
| | updateNote | Note! | | | ||
| | updateOrganization | Organization! | | | ||
| | updatePluginStatus | Plugin! | | | ||
| | updatePost | Post! | | | ||
| | updateSessionTimeout | Boolean! | | | ||
| | updateUserPassword | UserData! | | | ||
| | updateUserProfile | User! | | | ||
| | updateUserRoleInOrganization | Organization! | | | ||
| | updateUserTag | UserTag | | | ||
| | updateVolunteerMembership | VolunteerMembership! | | | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add access control directives and rate limiting documentation
The Mutation type contains sensitive operations that should be protected. Consider:
- Adding
@authdirectives to specify required permissions - Documenting rate limiting for mutations like
createDonation,sendMessageToChat - Adding security considerations to the field descriptions
Example:
type Mutation {
"""
Create a new donation.
Requires: ADMIN role
Rate limit: 10 requests per minute
"""
createDonation(input: DonationInput!): Donation! @auth(requires: ADMIN)
}| | Field | Type | Description | | ||
| |-------|------|-------------| | ||
| | _id | ID! | | | ||
| | actionItemCategory | ActionItemCategory | | | ||
| | allottedHours | Float | | | ||
| | assignee | EventVolunteer | | | ||
| | assigneeGroup | EventVolunteerGroup | | | ||
| | assigneeType | String! | | | ||
| | assigneeUser | User | | | ||
| | assigner | User | | | ||
| | assignmentDate | Date! | | | ||
| | completionDate | Date! | | | ||
| | createdAt | Date! | | | ||
| | creator | User | | | ||
| | dueDate | Date! | | | ||
| | event | Event | | | ||
| | isCompleted | Boolean! | | | ||
| | postCompletionNotes | String | | | ||
| | preCompletionNotes | String | | | ||
| | updatedAt | Date! | | |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add missing field descriptions
Many fields lack descriptions, which makes it harder for API consumers to understand their purpose. Consider adding meaningful descriptions for all fields.
Example:
| Field | Type | Description |
|-------|------|-------------|
-| _id | ID! | |
+| _id | ID! | Unique identifier for the action item |
-| actionItemCategory | ActionItemCategory | |
+| actionItemCategory | ActionItemCategory | Category classification for the action item |
-| allottedHours | Float | |
+| allottedHours | Float | Estimated time in hours to complete the action item |📝 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.
| | Field | Type | Description | | |
| |-------|------|-------------| | |
| | _id | ID! | | | |
| | actionItemCategory | ActionItemCategory | | | |
| | allottedHours | Float | | | |
| | assignee | EventVolunteer | | | |
| | assigneeGroup | EventVolunteerGroup | | | |
| | assigneeType | String! | | | |
| | assigneeUser | User | | | |
| | assigner | User | | | |
| | assignmentDate | Date! | | | |
| | completionDate | Date! | | | |
| | createdAt | Date! | | | |
| | creator | User | | | |
| | dueDate | Date! | | | |
| | event | Event | | | |
| | isCompleted | Boolean! | | | |
| | postCompletionNotes | String | | | |
| | preCompletionNotes | String | | | |
| | updatedAt | Date! | | | |
| | Field | Type | Description | | |
| |-------|------|-------------| | |
| | _id | ID! | Unique identifier for the action item | | |
| | actionItemCategory | ActionItemCategory | Category classification for the action item | | |
| | allottedHours | Float | Estimated time in hours to complete the action item | | |
| | assignee | EventVolunteer | The volunteer assigned to this action item | | |
| | assigneeGroup | EventVolunteerGroup | The volunteer group assigned to this action item | | |
| | assigneeType | String! | Type of assignee (individual or group) | | |
| | assigneeUser | User | The user assigned to this action item | | |
| | assigner | User | User who assigned this action item | | |
| | assignmentDate | Date! | Date when the action item was assigned | | |
| | completionDate | Date! | Date when the action item was completed | | |
| | createdAt | Date! | Timestamp when the action item was created | | |
| | creator | User | User who created this action item | | |
| | dueDate | Date! | Deadline for completing the action item | | |
| | event | Event | Associated event for this action item | | |
| | isCompleted | Boolean! | Flag indicating if the action item is completed | | |
| | postCompletionNotes | String | Notes added after completing the action item | | |
| | preCompletionNotes | String | Notes added before completing the action item | | |
| | updatedAt | Date! | Timestamp when the action item was last updated | |
|
Coderabbit is suggesting we do not use husky pre-commit hook for this since it will trigger a pipeline failure and therefore perform this via CI/CD. |
|
I am not quite sure why the |
|
This can be done much more simply.
|
|
|
|
yes I am working on this, And for the |
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.husky/pre-commit (1)
13-19:⚠️ Potential issueRemove documentation generation from pre-commit hook.
The documentation generation commands are causing pipeline failures due to unauthorized file modifications. This is a duplicate of the previous review comment.
As discussed in the PR:
- Move schema generation to CI/CD pipeline
- Use existing commands in package.json
- Consider implementing as a GitHub action
This will:
- Prevent pipeline failures
- Ensure consistent documentation
- Avoid blocking developer workflows
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized file modification or deletion attempt
🧹 Nitpick comments (1)
.husky/pre-commit (1)
Line range hint
1-24: Architectural recommendation for schema generation.Based on the PR discussion, here's a recommended approach:
- Create a GitHub Action that:
- Runs on the
develop-postgresbranch- Starts the server
- Generates schema documentation
- Commits and pushes changes
- Stops the server
Would you like me to help create a GitHub Action workflow for this automation?
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized file modification or deletion attempt
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.husky/pre-commit(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: PR Workflow
.husky/pre-commit
[error] Unauthorized file modification or deletion attempt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (typescript)
.husky/pre-commit
Outdated
| npm run dev & | ||
| SERVER_PID=$! | ||
|
|
||
| # Checks code for typescript type errors and throws errors if found. | ||
| npm run typecheck | ||
| # Wait for server to be ready (adjust sleep time as needed) | ||
| sleep 10 |
There was a problem hiding this comment.
Remove server management from pre-commit hook.
Starting a server in a pre-commit hook introduces several risks:
- Race conditions if multiple developers commit simultaneously
- Potential port conflicts if a server is already running
- Unreliable sleep-based timing
- Resource overhead for every commit
Based on the PR discussion with @palisadoes, this should be moved to a CI/CD workflow instead.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized file modification or deletion attempt
.husky/pre-commit
Outdated
| # Kill the background server process | ||
| kill $SERVER_PID |
There was a problem hiding this comment.
Remove server cleanup code.
The server cleanup is tightly coupled with the server startup, which should be removed from the pre-commit hook.
If the server fails to start or the script exits prematurely, this could kill unrelated processes or leave orphaned processes.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized file modification or deletion attempt
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
.husky/pre-commit (3)
6-10:⚠️ Potential issueRemove server management from pre-commit hook.
Starting a server in a pre-commit hook introduces several risks:
- Race conditions if multiple developers commit simultaneously
- Port conflicts if a server is already running
- Unreliable sleep-based timing
- Resource overhead for every commit
Based on the PR discussion with @palisadoes, this should be moved to a CI/CD workflow instead.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized modification or deletion of protected file '.husky/pre-commit'. This file is restricted from changes.
18-19:⚠️ Potential issueRemove documentation generation from pre-commit hook.
The GraphQL documentation generation commands modify git-tracked files, causing pipeline failures with "Unauthorized file modification" errors. Documentation generation should be moved to the CI/CD pipeline to:
- Prevent commit failures due to unauthorized file modifications
- Ensure consistent documentation across all branches
- Avoid race conditions during concurrent commits
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized modification or deletion of protected file '.husky/pre-commit'. This file is restricted from changes.
21-21:⚠️ Potential issueRemove server cleanup code.
The server cleanup is tightly coupled with the server startup, which should be removed from the pre-commit hook. If the server fails to start or the script exits prematurely, this could kill unrelated processes or leave orphaned processes.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized modification or deletion of protected file '.husky/pre-commit'. This file is restricted from changes.
🧹 Nitpick comments (1)
.husky/pre-commit (1)
6-21: Move schema generation to GitHub Actions workflow.Based on the PR discussion, here's a recommended approach:
- Create a new GitHub Action workflow that:
- Starts the server
- Waits for server health check
- Generates schema and documentation
- Commits and pushes changes if needed
- Stops the server
- Remove all server management and documentation generation from pre-commit hook
- Keep only local development concerns in pre-commit (linting, formatting, type checking)
This approach will:
- Ensure reliable and consistent schema generation
- Prevent pipeline failures
- Reduce complexity in local development workflow
Would you like me to help create this GitHub Action workflow?
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized modification or deletion of protected file '.husky/pre-commit'. This file is restricted from changes.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
.husky/pre-commit(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🪛 GitHub Actions: PR Workflow
.husky/pre-commit
[error] Unauthorized modification or deletion of protected file '.husky/pre-commit'. This file is restricted from changes.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (typescript)
|
Please work with @gautam-divyanshu and @tasneemkoushar to get this PR merged. |
|
Sure I would like to collaborate with them to come up with a better approach, however I have been facing this issue with |
|
@Suyash878 npm error |
|
Do you think I should proceed with this as a github action? jobs: |
|
The best way to go about this issue is to use CI/CD as it is a consistent and reliable in comparison to husky's pre-commit approach which has a lot of caveats to it. |
Comparison of the Two Options
These are two simple ways in which we can start the local server for schema generation, I feel we should prefer using localhost as it is faster less heavier compared to the docker approach. |
What kind of change does this PR introduce?
Feature
Issue Number:
Fixes #2818
Snapshots/Videos:
NA
If relevant, did you update the documentation?
Not sure.
Summary
I have added a new
schema-script.mjswhich automates the generation of schema.json and schema.md.Does this PR introduce a breaking change?
NA
Checklist
CodeRabbit AI Review
Test Coverage
Other information
NA
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Release Notes
Chores
Documentation
Build
These changes enhance project development processes and documentation, providing better clarity and workflow management.