[Refactor]:simplify version info printing logic by feyounger · Pull Request #5032 · volcano-sh/volcano

feyounger · 2026-02-08T10:31:46Z

What type of PR is this?

Refactor / Code cleanup

What this PR does / why we need it:

This PR refactors the version printing and information generation logic to improve code readability, simplicity, and maintainability.

Key improvements include:
• Removing unnecessary parameter passing for apiVersion
• Simplifying output formatting using idiomatic Go patterns
• Cleaning up redundant variables and improving function signatures

There is no functional or behavior change introduced by this refactor.

Which issue(s) this PR fixes:

Fixes #5031

Special notes for your reviewer:

This is a non-functional refactor only.
No behavior change or user-facing impact is introduced.

Does this PR introduce a user-facing change?

NONE

volcano-sh-bot · 2026-02-08T10:31:52Z

[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 archlitchi 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:

OWNERS

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

gemini-code-assist · 2026-02-08T10:31:58Z

Summary of Changes

Hello @feyounger, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on refactoring the version information printing logic within the 'pkg/version' package. The primary goal is to enhance code readability, simplify the implementation, and improve maintainability by removing an unnecessary parameter and streamlining output formatting. These changes are purely internal and do not alter any user-facing behavior or functionality, addressing issue #5031.

Highlights

Parameter Removal: The 'apiVersion' parameter has been removed from the 'Info()' function and its call site in 'PrintVersionAndExit()', as it was determined to be unnecessary for its operation.
Output Formatting Simplification: The 'PrintVersionAndExit()' function now utilizes 'fmt.Println()' for output, replacing the more verbose 'fmt.Printf("%v\n", i)' for cleaner and more idiomatic Go code.
Code Readability and Maintainability: Overall code readability and maintainability of the version printing logic have been significantly improved through these refactoring efforts, aligning with best practices.
No Functional Change: This pull request is a pure refactor and introduces no functional or behavioral changes to the application, ensuring existing functionality remains intact.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

pkg/version/version.go
- The 'PrintVersionAndExit' function's call to 'Info' no longer passes the 'apiVersion' parameter.
- The 'PrintVersionAndExit' function now uses 'fmt.Println' for output, replacing 'fmt.Printf("%v\n", i)'.
- The 'Info' function's signature has been updated to remove the 'apiVersion string' parameter.

Activity

The pull request was created by feyounger.
The PR addresses issue Refactor: simplify version info generation and printing logic #5031.
The author explicitly stated that this is a non-functional refactor with no behavior change or user-facing impact.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request refactors the version printing logic, which improves code clarity. However, it introduces a breaking change by modifying the signature of an exported function Info. This will break any external projects that rely on this function's signature. I have added a comment recommending to preserve backward compatibility if this package is used as a library.

gemini-code-assist · 2026-02-08T10:32:59Z

pkg/version/version.go


 // Info returns an array of various service versions.
-func Info(apiVersion string) []string {
+func Info() []string {


Changing the signature of the exported function Info is a breaking API change. This will break any external projects that import this package and call this function. If this package is consumed as a library, this change should be reverted to maintain backward compatibility. A new function with the desired signature could be introduced instead if needed.

Please create a new function instead.

feyounger · 2026-02-08T14:25:47Z

Looks like a flaky e2e failure related to job lifecycle handling.
This PR only contains refactoring changes without functional impact.
Could you please help re-run the CI?

hajnalmt

/priority low

We shouldn't break the existing API.
Please mark it as deprecated with a comment and create a new function. Also please don't forget to sign the DCO.

hajnalmt · 2026-02-11T12:58:53Z

pkg/version/version.go


 // Info returns an array of various service versions.
-func Info(apiVersion string) []string {
+func Info() []string {


Please create a new function instead.

Copilot

Pull request overview

Refactors the pkg/version package’s version info printing/generation to reduce redundant formatting/parameter passing and make output more idiomatic.

Changes:

Updated version printing to use fmt.Println for line-by-line output.
Introduced GetInfo() helper and adjusted Info() to delegate to it.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-12T01:58:48Z

pkg/version/version.go

+	return GetInfo()
+}
+
+func GetInfo() []string {


Info(apiVersion string) now ignores its apiVersion argument and always returns info built from the package-level apiVersion. This is a behavior change for any callers that passed a custom value and also makes the signature misleading. Either remove the parameter (and update callers) or preserve backward compatibility by using the passed-in apiVersion when constructing the output (e.g., have the helper accept an argument).

Suggested change

return GetInfo()

}

func GetInfo() []string {

return buildInfo(apiVersion)

}

func GetInfo() []string {

return buildInfo(apiVersion)

}

func buildInfo(apiVersion string) []string {

Copilot · 2026-02-12T01:58:49Z

pkg/version/version.go

+func GetInfo() []string {
 	return []string{


GetInfo is introduced as a new exported API but is only used internally in this package. Consider making it unexported (e.g., getInfo) to avoid expanding the public surface area, or document clearly why a new exported function is needed.

Copilot · 2026-02-12T01:58:49Z

pkg/version/version.go

+func GetInfo() []string {
 	return []string{
-		fmt.Sprintf("API Version: %s", apiVersion),
+		fmt.Sprintf("API Version: %s", apiVersion), // 假设 apiVersion 已经在包级别定义了


The inline comment is in Chinese and reads like an implementation assumption. To keep comments consistent and maintainable for all contributors, please remove it or rewrite it in concise English (ideally as a proper doc comment if it’s important).

Suggested change

fmt.Sprintf("API Version: %s", apiVersion), // 假设 apiVersion 已经在包级别定义了

fmt.Sprintf("API Version: %s", apiVersion),

We can remove this whole line I think.

Copilot · 2026-02-12T01:58:49Z

pkg/version/version.go

@@ -34,16 +34,20 @@ var (

 // PrintVersionAndExit prints versions from the array returned by Info() and exit.


The doc comment says this prints versions returned by Info(), but the implementation now iterates over GetInfo(). Please update the comment to match the current behavior (or switch back to calling Info if that’s the intended API).

Suggested change

// PrintVersionAndExit prints versions from the array returned by Info() and exit.

// PrintVersionAndExit prints versions from the array returned by GetInfo() and exits.

Copilot is right here, we need to modify this comment now.

hajnalmt

Hello,
On a second thought, I tried to check around the code, and we don't set apiVersion anywhere and this is invoked only by us so it's safe to remove apiVersion in version.go everywhere. Sorry for the double round.
Please remove the variable in version.go file too, and we can stay with the old Info function name I think.

hajnalmt · 2026-02-12T13:52:03Z

pkg/version/version.go

+func GetInfo() []string {
 	return []string{
-		fmt.Sprintf("API Version: %s", apiVersion),
+		fmt.Sprintf("API Version: %s", apiVersion), // 假设 apiVersion 已经在包级别定义了


We can remove this whole line I think.

hajnalmt · 2026-02-12T13:55:26Z

pkg/version/version.go

@@ -34,16 +34,20 @@ var (

 // PrintVersionAndExit prints versions from the array returned by Info() and exit.


Copilot is right here, we need to modify this comment now.

Signed-off-by: Aniket <[email protected]> Signed-off-by: feyounger <[email protected]>

Signed-off-by: caotuo721 <[email protected]> # Conflicts: # pkg/webhooks/admission/jobs/validate/admit_job_test.go Signed-off-by: feyounger <[email protected]>

Signed-off-by: feyounger <[email protected]>

feyounger · 2026-02-12T15:04:59Z

Hello, On a second thought, I tried to check around the code, and we don't set apiVersion anywhere and this is invoked only by us so it's safe to remove apiVersion in version.go everywhere. Sorry for the double round. Please remove the variable in version.go file too, and we can stay with the old Info function name I think.

Okay

volcano-sh-bot requested review from Thor-wl and wangyang0616 February 8, 2026 10:31

volcano-sh-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 8, 2026

gemini-code-assist bot reviewed Feb 8, 2026

View reviewed changes

hajnalmt suggested changes Feb 11, 2026

View reviewed changes

volcano-sh-bot added the priority/low label Feb 11, 2026

Copilot AI review requested due to automatic review settings February 12, 2026 01:48

volcano-sh-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 12, 2026

Copilot started reviewing on behalf of feyounger February 12, 2026 01:49 View session

feyounger force-pushed the 5031 branch 2 times, most recently from b9c2df5 to a390eef Compare February 12, 2026 01:57

Copilot AI reviewed Feb 12, 2026

View reviewed changes

feyounger force-pushed the 5031 branch 2 times, most recently from 3878b70 to 022cb8b Compare February 12, 2026 05:04

hajnalmt reviewed Feb 12, 2026

View reviewed changes

volcano-sh-bot added do-not-merge/contains-merge-commits size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 12, 2026

aniketchawardol and others added 5 commits February 12, 2026 23:03

fix typos as per suggestions in PR 472 in website

1499ed9

Signed-off-by: Aniket <[email protected]> Signed-off-by: feyounger <[email protected]>

Support Pod-level Resource Request and Limit Settings

83c229f

Signed-off-by: caotuo721 <[email protected]> # Conflicts: # pkg/webhooks/admission/jobs/validate/admit_job_test.go Signed-off-by: feyounger <[email protected]>

[Refactor]:simplify version info printing logic

135362d

Signed-off-by: feyounger <[email protected]>

feat: add GetInfo and deprecate Info

6fcac50

Signed-off-by: feyounger <[email protected]>

feat: restore Info()

d913ba3

Signed-off-by: feyounger <[email protected]>

feyounger force-pushed the 5031 branch from 9eaf3ec to d913ba3 Compare February 12, 2026 15:03

volcano-sh-bot removed do-not-merge/contains-merge-commits size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 12, 2026

volcano-sh-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 12, 2026

	fmt.Sprintf("API Version: %s", apiVersion), // 假设 apiVersion 已经在包级别定义了
	fmt.Sprintf("API Version: %s", apiVersion),

		@@ -34,16 +34,20 @@ var (

		// PrintVersionAndExit prints versions from the array returned by Info() and exit.

Conversation

feyounger commented Feb 8, 2026

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

volcano-sh-bot commented Feb 8, 2026

Uh oh!

gemini-code-assist bot commented Feb 8, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

hajnalmt Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

feyounger commented Feb 8, 2026

Uh oh!

hajnalmt left a comment

Choose a reason for hiding this comment

Uh oh!

hajnalmt Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

hajnalmt Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

hajnalmt Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

hajnalmt left a comment

Choose a reason for hiding this comment

Uh oh!

hajnalmt Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

hajnalmt Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

feyounger commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments