Skip to content

fix(makefile): remove stale clean target cleanup#330

Open
mrinalchaturvedi27 wants to merge 2 commits into
volcano-sh:mainfrom
mrinalchaturvedi27:fix-clean-target
Open

fix(makefile): remove stale clean target cleanup#330
mrinalchaturvedi27 wants to merge 2 commits into
volcano-sh:mainfrom
mrinalchaturvedi27:fix-clean-target

Conversation

@mrinalchaturvedi27
Copy link
Copy Markdown

@mrinalchaturvedi27 mrinalchaturvedi27 commented May 13, 2026

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This updates make clean so it removes the Go binary output and Python package build artifacts. It also adds the clean target to make help.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

No generated files changed. I checked the target with make -n clean, make clean, and make help.

Does this PR introduce a user-facing change?:

NONE

What I fixed

I fixed make clean.

The Makefile builds Go binaries inside bin/, and the Python SDK and CLI build targets write packages to sdk-python/dist/ and cmd/cli/dist/. The old clean target removed only bin/.

How I checked it

I checked the Makefile build targets and confirmed the Go and Python build output paths.

What I changed

I removed the unused root-level rm -f line, added a help description for clean, and included the Python package dist/ folders in cleanup.

Why this is okay

This is a small cleanup fix. The target now matches the actual build outputs and appears in make help.

Validation

  • make -n clean - Passed.
  • make clean - Passed.
  • make help - Passed; clean is listed with its description.
  • DCO check - Passed; both commits have Signed-off-by lines.

Signed-off-by: Mrinal Chaturvedi <mrinal.chaturvedi27@gmail.com>
Copilot AI review requested due to automatic review settings May 13, 2026 10:55
@volcano-sh-bot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kevin-wangzefeng for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request simplifies the clean target in the Makefile by removing specific binary deletions. Review feedback suggests adding a help description to the target for consistency and expanding the cleanup command to include build artifacts from the Python SDK and CLI directories.

Comment thread Makefile Outdated
Comment thread Makefile Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes a stale root-level binary cleanup step from make clean, aligning the clean target with the actual build output location (bin/) used by AgentCube’s Makefile-based build workflow.

Changes:

  • Drop rm -f workloadmanager agentd agentcube-router from the clean target since builds already output to bin/.
  • Keep make clean focused on removing bin/, which contains the built artifacts produced by go build -o bin/....

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 13, 2026

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.12%. Comparing base (524e55e) to head (574fb91).
⚠️ Report is 54 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #330      +/-   ##
==========================================
+ Coverage   47.57%   49.12%   +1.55%     
==========================================
  Files          30       30              
  Lines        2819     2858      +39     
==========================================
+ Hits         1341     1404      +63     
+ Misses       1338     1301      -37     
- Partials      140      153      +13     
Flag Coverage Δ
unittests 49.12% <ø> (+1.55%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Mrinal Chaturvedi <mrinal.chaturvedi27@gmail.com>
@mrinalchaturvedi27
Copy link
Copy Markdown
Author

Hi @hzxuzhonghu @YaoZengzeng quick ping for this PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants