Skip to content

fix: remove duplicate status check and add missing port flag description in start command#437

Closed
A-d-i-t-y wants to merge 2 commits into
microcks:masterfrom
A-d-i-t-y:fix/start-command-improvements
Closed

fix: remove duplicate status check and add missing port flag description in start command#437
A-d-i-t-y wants to merge 2 commits into
microcks:masterfrom
A-d-i-t-y:fix/start-command-improvements

Conversation

@A-d-i-t-y
Copy link
Copy Markdown

@A-d-i-t-y A-d-i-t-y commented May 25, 2026

Summary

Two small but genuine fixes in cmd/start.go.

Fix 1 — Remove duplicate Running status check

The Running status was being checked twice:

  1. First via an explicit if block before the switch
  2. Then again as a case inside the switch statement

The duplicate check was redundant and has been removed.

Fix 2 — Add missing --port flag description

The --port flag had an empty description string:
startCmd.Flags().StringVar(&hostPort, "port", "8585", "")

Users running microcks start --help had no idea what --port does.
Added a proper description.

Files Changed

  • cmd/start.go — removed duplicate check, added port flag description

@A-d-i-t-y
fix: remove duplicate status check and add missing port flag descript…
4b675e4 
…ion in start command

- Removed duplicate 'Running' status check that was already
  handled by the switch statement below
- Added missing description for --port flag so users understand
  what the flag does

Signed-off-by: Aditya <aaaditya1909@gmail.com>
@A-d-i-t-y
Copy link
Copy Markdown
Author

A-d-i-t-y commented May 25, 2026

Hi @Harsh4902 @Vaishnav88sk, I have raised this PR to fix two small issues in the start command - removed a duplicate Running status check and added a missing description for the --port flag. Would love your feedback!

@Vaishnav88sk
Copy link
Copy Markdown

Vaishnav88sk commented May 25, 2026

I think it's a duplicate of #307
https://github.com/microcks/microcks-cli/pull/307/changes#diff-e3ad76770e5ac8ed654e8c5585782469e67e0d981adb9153604109263f22e93e

@A-d-i-t-y Please check!

@A-d-i-t-y
Copy link
Copy Markdown
Author

A-d-i-t-y commented May 25, 2026

Hi @Vaishnav88sk, thank you for pointing this out! I apologize for the oversight — I was not aware that this was already covered in PR #307. I will close this PR now. Sorry for the noise!

@A-d-i-t-y A-d-i-t-y closed this May 25, 2026
@Vaishnav88sk
Copy link
Copy Markdown

Vaishnav88sk commented May 25, 2026

Hi @Vaishnav88sk, thank you for pointing this out! I apologize for the oversight — I was not aware that this was already covered in PR #307. I will close this PR now. Sorry for the noise!

Not to worry, and please do not say sorry. Just check if it exists for next time. 👍🏻

@A-d-i-t-y
Copy link
Copy Markdown
Author

A-d-i-t-y commented May 25, 2026

Thank you @Vaishnav88sk! Will make sure to check existing PRs before raising new ones. Appreciated

Also, I have 5 other PRs open whenever you get a chance to review:

No rush at all, happy to make any changes based on your feedback

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

Reviewers

@lbroudoux lbroudoux Awaiting requested review from lbroudoux lbroudoux is a code owner

@yada yada Awaiting requested review from yada yada is a code owner

@Harsh4902 Harsh4902 Awaiting requested review from Harsh4902 Harsh4902 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@A-d-i-t-y @Vaishnav88sk