refactor(internal/librarianops): move ParseRepoFlags to common folder

[miguelvelezsa]

[3 of 4]
Move ParseRepoFlags function to a common folder so we can use it in other commands.
Right now needed for upgrade command #3911.

[gemini-code-assist]

Code Review

This pull request is a good refactoring that moves the ParseRepoFlags function to a common package for reuse across different commands. My review includes a couple of suggestions to further improve the implementation. First, I've recommended renaming the new package to align with Go's idiomatic naming conventions. Second, I've suggested refactoring ParseRepoFlags to be a pure function by removing its side effect on a global variable. This will make the function's behavior more explicit and improve testability.

[gemini-code-assist]

This function modifies the global variable command.Verbose, which is a side effect that can make the code harder to understand and test. The test file for this function already needs a t.Cleanup to manage this global state.

To improve this, ParseRepoFlags should be a pure function. It should return the value of the verbose flag to the caller, which can then decide to set the global variable. This makes the side effect explicit at the call site and simplifies testing. This aligns with the principle of making mutations explicit, as described in our style guide.

Reference:

• How We Write Go: "When a function modifies a pointer parameter, return the modified value to make the mutation explicit. This makes it so that functions are clear about their side effects."

Suggested change

	func ParseRepoFlags(cmd *cli.Command) (repoName, workDir string, err error) {
	workDir = cmd.String("C")
	command.Verbose = cmd.Bool("v")
	if workDir != "" {
	// When -C is provided, infer repo name from directory basename.
	repoName = filepath.Base(workDir)
	} else {
	// When -C is not provided, require positional repo argument.
	if cmd.Args().Len() == 0 {
	return "", "", fmt.Errorf("usage: librarianops <command> <repo> or librarianops <command> -C <dir>")
	}
	repoName = cmd.Args().Get(0)
	}
	return repoName, workDir, nil
	}
	func ParseRepoFlags(cmd *cli.Command) (repoName, workDir string, verbose bool, err error) {
	workDir = cmd.String("C")
	verbose = cmd.Bool("v")
	if workDir != "" {
	// When -C is provided, infer repo name from directory basename.
	repoName = filepath.Base(workDir)
	} else {
	// When -C is not provided, require positional repo argument.
	if cmd.Args().Len() == 0 {
	return "", "", false, fmt.Errorf("usage: librarianops <command> <repo> or librarianops <command> -C <dir>")
	}
	repoName = cmd.Args().Get(0)
	}
	return repoName, workDir, verbose, nil
	}

References

1. When a function modifies a parameter, it should return the modified value to make the mutation explicit. This principle extends to other side effects like modifying global variables. ^(link)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.87%. Comparing base (cf3e6f1) to head (f8af799).

Additional details and impacted files

@@                   Coverage Diff                   @@
##           upgrade-config-updater    #3927   +/-   ##
=======================================================
  Coverage                   81.87%   81.87%           
=======================================================
  Files                          73       74    +1     
  Lines                        6285     6285           
=======================================================
  Hits                         5146     5146           
  Misses                        791      791           
  Partials                      348      348           

☔ 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.

[julieqiu]

Same comment as on the other PRs — I’d keep this where it is. We want to avoid creating lots of tiny packages, since that adds maintenance overhead without clear benefit.

https://dave.cheney.net/practical-go/presentations/qcon-china.html#_package_design is a good read.