feat: include sandbox type when listing partner sandboxes#511
Conversation
vegeris
added
semver:patch
vegeris
force-pushed
the
evegeris-highlight-partner-sandboxes
branch
from
c59ebff to
b58c106
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #511 +/- ##
==========================================
- Coverage 71.19% 71.18% -0.01%
==========================================
Files 222 222
Lines 18678 18680 +2
==========================================
+ Hits 13297 13298 +1
- Misses 4200 4201 +1
Partials 1181 1181 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
srtaalej
left a comment
srtaalej
left a comment
There was a problem hiding this comment.
thank you for these updates ⭐ i left a comment about the output - i can make the change if you agree 💟
| sandboxType := "Regular" | ||
| if s.IsPartner { | ||
| sandboxType = "Partner" | ||
| } | ||
| clients.IO.PrintInfo(ctx, false, " %s", style.Secondary(fmt.Sprintf("Type: %s", sandboxType))) | ||
|
|
There was a problem hiding this comment.
How would you feel about making the Type param optional? so that it only shows when there is a Partner sandbox 🤔
since most sandboxes are regular the Type field adds visual noise to the output. so it might be helpful to only differentiate when there exists a partner sandbox
There was a problem hiding this comment.
Sounds good to me 🙂 Particularly with Partners being a small percentage of developers
| } | ||
| clients.IO.PrintInfo(ctx, false, " %s", style.Secondary(fmt.Sprintf("Type: %s", sandboxType))) | ||
| } | ||
|
|
There was a problem hiding this comment.
Thanks for making this update!
From your earlier comment I was thinking we would want to only add this label if isPartner is true:
if s.IsPartner {
clients.IO.PrintInfo(ctx, false, " %s", style.Secondary("Type: Partner"))
}
(The API will return the is_partner param set to false for regular sandboxes)
There was a problem hiding this comment.
hmm yes i guess my comment wasnt very clear - your version is simpler though so i'll go ahead and change it!
vegeris
changed the title
vegeris
force-pushed
the
evegeris-highlight-partner-sandboxes
branch
from
43d0ccb to
c376e4a
Compare
3 participants
Changelog
This does not need to be included in the Changelog
Summary
This PR updates the Sandbox model to include the 'is_partner' bool which indicates whether this is a regular sandbox or a partner sandbox, and display it when listing one's partner sandboxes. We do not display the 'type' label for regular sandboxes to reduce visual noise
Preview
Testing
% hermes sandbox list --experiment=sandboxesRequirements