Skip to content
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.

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

cmd: replace args with specific argument(s) in helper functions #1075

Open
elhimov opened this issue Dec 26, 2024 · 0 comments
Open

cmd: replace args with specific argument(s) in helper functions #1075

elhimov opened this issue Dec 26, 2024 · 0 comments
Assignees
Labels
code health Improve code readability, simplify maintenance and so on teamE

Comments

@elhimov
Copy link
Contributor

elhimov commented Dec 26, 2024

Argument args looks natural in command handler functions where its meaning is quite clear. A few examples:

func internalConnectModule(cmdCtx *cmdcontext.CmdCtx, args []string) error
func internalRestartModule(cmdCtx *cmdcontext.CmdCtx, args []string) error

But being used in helper functions, like

func replicasetFillCtx(cmdCtx *cmdcontext.CmdCtx, ctx *replicasetCtx, args []string,
	isRunningCtxRequired bool) error
func resolveConnectOpts(cmdCtx *cmdcontext.CmdCtx, cliOpts *config.CliOpts,
	connectCtx *connect.ConnectCtx, args []string) ...

it turns into a problem, because this kind of functions expect some specific argument(s), not the abstract list of arguments. It makes the code less readable and harder to maintain.

@elhimov elhimov self-assigned this Dec 26, 2024
@elhimov elhimov added code health Improve code readability, simplify maintenance and so on teamE labels Dec 26, 2024
elhimov added a commit that referenced this issue Dec 26, 2024
Namely this commit fixes:
- resolveConnectOpts
- replicasetFillCtx

Part of #1075
elhimov added a commit that referenced this issue Dec 26, 2024
Namely this commit fixes:
- resolveConnectOpts
- replicasetFillCtx

Part of #1075
elhimov added a commit that referenced this issue Dec 27, 2024
Namely this commit fixes:
- resolveConnectOpts
- replicasetFillCtx

Part of #1075
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code health Improve code readability, simplify maintenance and so on teamE
Projects
None yet
Development

No branches or pull requests

1 participant