Skip to content

feat: Add id to profile create output#394

Open
bobbyhouse wants to merge 2 commits intomainfrom
add-id-to-profile-output
Open

feat: Add id to profile create output#394
bobbyhouse wants to merge 2 commits intomainfrom
add-id-to-profile-output

Conversation

@bobbyhouse
Copy link
Copy Markdown
Contributor

What I did
Add output options to the profile create command so that we can return an id for the newly create profile. Supported formats are consistent with other commands (i.e. yaml and json)

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did

Add output options to the profile create command so that we can
return an id for the newly create profile. Supported formats
are consistent with other commands (i.e. yaml and json)
@bobbyhouse bobbyhouse force-pushed the add-id-to-profile-output branch from f36307e to bca20a0 Compare February 10, 2026 19:02
@bobbyhouse bobbyhouse marked this pull request as ready for review February 10, 2026 19:09
@bobbyhouse bobbyhouse requested a review from a team as a code owner February 10, 2026 19:09
Copy link
Copy Markdown
Contributor

@cmrigney cmrigney left a comment

Choose a reason for hiding this comment

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

lgtm. Just left a couple chore suggestions. I could be wrong but I think we already test those cases.

Comment on lines +92 to +97
case OutputFormatJSON:
// Output JSON with just the profile ID
fmt.Printf("{\"id\":\"%s\"}\n", id)
case OutputFormatYAML:
// Output YAML with just the profile ID
fmt.Printf("id: %s\n", id)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thought: We don't allow quotes in ids, so this seems safe. But if we wanted to future proof it, we could do something like:

var o struct {
  ID string `yaml:"id" json:"id"`
}
o.ID = id
data, err := json.Marshal(o)
// err check...
fmt.Printf("%s\n", string(data))

}

// TestCreateJSONWithDuplicateIDPrevention tests that JSON output reflects the actual ID with duplicate suffix
func TestCreateJSONWithDuplicateIDPrevention(t *testing.T) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

chore: We already test ID duplication I think, so we can remove this test.

}

// TestCreateJSONWithNoServers tests that JSON output works correctly with empty server list
func TestCreateJSONWithNoServers(t *testing.T) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

chore: This might be overkill since this behavior is already tested I think, and we're really just worried about testing output format.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants