Skip to content

ci: upstream change #4263

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.

Sign up for GitHub

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

Closed
ruthra-kumar wants to merge 1 commit into from
+2 −1
Closed

ci: upstream change #4263

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/helper/install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ sed -i 's/socketio:/# socketio:/g' Procfile
sed -i 's/redis_socketio:/# redis_socketio:/g' Procfile

bench get-app "https://github.com/${frappeuser}/payments" --branch "$paymentsbranch"
bench get-app "https://github.com/${frappeuser}/erpnext" --branch "$erpnextbranch" --resolve-deps
# bench get-app "https://github.com/${frappeuser}/erpnext" --branch "$erpnextbranch" --resolve-deps
bench get-app "https://github.com/ruthra-kumar/erpnext" --branch "move_commits_inside_test_guard_clause" --resolve-deps
Comment on lines +51 to +52
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Debug code should not be merged: Hardcoded personal fork and test branch.

This change replaces the parameterized ERPNext source with a hardcoded personal fork (ruthra-kumar/erpnext) and a test-specific branch (move_commits_inside_test_guard_clause). This appears to be temporary test code that should not be committed to the develop branch:

  1. Bypasses CI parameterization: The $frappeuser and $erpnextbranch variables (lines 14, 16) are now ignored for ERPNext, breaking the FRAPPE_USER/ERPNEXT_BRANCH environment variable support used by ci.yml and run-individual-tests.yml.

  2. Creates inconsistency: Frappe (line 20), payments (line 50), and lending (line 53) still use parameterized URLs, while ERPNext now uses a hardcoded fork.

  3. Branch name indicates test code: The branch name move_commits_inside_test_guard_clause suggests this is for testing specific changes, not a permanent CI configuration.

If you need to test against a fork temporarily, consider using the existing FRAPPE_USER mechanism or reverting before merge.

Suggested fix: Revert to parameterized command 
-# bench get-app "https://github.com/${frappeuser}/erpnext" --branch "$erpnextbranch" --resolve-deps
-bench get-app "https://github.com/ruthra-kumar/erpnext" --branch "move_commits_inside_test_guard_clause" --resolve-deps
+bench get-app "https://github.com/${frappeuser}/erpnext" --branch "$erpnextbranch" --resolve-deps
📝 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.

Suggested change
# bench get-app "https://github.com/${frappeuser}/erpnext" --branch "$erpnextbranch" --resolve-deps
bench get-app "https://github.com/ruthra-kumar/erpnext" --branch "move_commits_inside_test_guard_clause" --resolve-deps
bench get-app "https://github.com/${frappeuser}/erpnext" --branch "$erpnextbranch" --resolve-deps
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/helper/install.sh around lines 51 - 52, The bench get-app invocation
was changed to a hardcoded fork/branch; revert it to use the parameterized
variables so CI/env vars work. Replace the hardcoded bench get-app line with the
parameterized form that uses $frappeuser (or FRAPPE_USER) and $erpnextbranch (or
ERPNEXT_BRANCH) so ERPNext is installed consistently like the other apps; update
the bench get-app call (the one currently calling "ruthra-kumar/erpnext" and
branch "move_commits_inside_test_guard_clause") to the original parameterized
invocation that references $frappeuser and $erpnextbranch.

bench get-app "https://github.com/${frappeuser}/lending" --branch "$lendingbranch"
bench get-app hrms "${GITHUB_WORKSPACE}"
bench setup requirements --dev
Expand Down
Loading