fix: added fix for commit messages #39134
Conversation
WalkthroughThis update refines the commit request process in the git module. The commitRequestOld function now constructs a payload that includes only the commitMessage and doPush fields rather than the entire params object. Additionally, the commitRequest function has been updated to reflect the new parameter name (changed from branchedApplicationId to refArtifactId) in commitRequestOld. The control flow based on the isNew flag remains unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Caller
participant CR as commitRequest
participant CO as commitRequestOld
participant CN as commitRequestNew
participant API as Api.post
C->>CR: commitRequest(artifactType, refArtifactId, params, isNew)
alt isNew true
CR->>CN: commitRequestNew(artifactType, refArtifactId, params)
CN->>API: Api.post(filtered payload)
else isNew false
CR->>CO: commitRequestOld(refArtifactId, params)
Note right of CO: Constructs payload with commitMessage & doPush
CO->>API: Api.post(filtered payload)
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13199264265. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/git/requests/commitRequest.ts (1)
31-41: Ensure parameter naming consistency across functionsThe main function uses
refArtifactIdbut passes it tocommitRequestOldwhich expectsbranchedApplicationId. While this works functionally, the parameter naming should be consistent.Consider updating the parameter names to be consistent across all functions:
-async function commitRequestOld( - branchedApplicationId: string, +async function commitRequestOld( + refArtifactId: string,
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/git/requests/commitRequest.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
🔇 Additional comments (2)
app/client/src/git/requests/commitRequest.ts (2)
10-13: Parameter name inconsistency with AI summaryThe AI summary indicates the parameter name was changed to
refArtifactId, but the implementation still usesbranchedApplicationId. This inconsistency should be addressed.Please clarify if the parameter name should be updated to match the summary.
14-17: LGTM! Improved payload structureGood improvement to only send the required fields (
commitMessageanddoPush) instead of the entire params object.
github-actions
bot
added
Git Product
|
Deploy-Preview-URL: https://ce-39134.dp.appsmith.com |
Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Fixes #39115
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Git"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13199336570
Commit: 62eff0c
Cypress dashboard.
Tags:
@tag.GitSpec:
Fri, 07 Feb 2025 14:07:19 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit