feat: output a list of samples for a language with the samples command #192
Conversation
zimeg
added
changelog
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #192 +/- ##
==========================================
+ Coverage 62.95% 63.00% +0.05%
==========================================
Files 212 212
Lines 21782 21848 +66
==========================================
+ Hits 13713 13766 +53
- Misses 7006 7018 +12
- Partials 1063 1064 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zimeg
left a comment
zimeg
left a comment
There was a problem hiding this comment.
🌚 Quick notes for amazing reviewers follow!
| if err != nil { | ||
| return err | ||
| } | ||
| if samplesListFlag || !clients.IO.IsTTY() { |
There was a problem hiding this comment.
👁️🗨️ note: We avoid attempting prompts in a non-interactive environment and instead list samples!
🗣️ note: The changes of #193 make this behavior more clear IMO since flag substitutes will then have matching prompts or the create command should be used.
mwbrooks
left a comment
mwbrooks
left a comment
There was a problem hiding this comment.
📝 Nice feature! I personally enjoy the flat list more than the interactive one.
❓ Curious where this feature idea came from?
🧪 While testing, I noticed that the order isn't sorted on my side. Is that happening on your side too? Also, I left a suggestion to remove the emojis since they don't have a strong meaning.
| // listSampleSelection outputs available samples matching a language flag filter | ||
| func listSampleSelection(ctx context.Context, clients *shared.ClientFactory, sampleRepos []create.GithubRepo) error { | ||
| filteredRepos := filterRepos(sampleRepos, samplesLanguageFlag) | ||
| sortedRepos := sortRepos(filteredRepos) |
There was a problem hiding this comment.
question: Are you noticing that the repos aren't sorted?
There was a problem hiding this comment.
@mwbrooks This confused me too but these might find order from the stars? ⭐
I wonder if showing the number of stars would be helpful? I might expect this to prefix the sample, but am not sure at all if that'd be right:
⭐ 60 - deno-timesheet-approval
Collect timesheet information from users and store it in a Google Sheet
https://github.com/slack-samples/deno-timesheet-approval
cmd/project/samples.go
Outdated
| emojis := []string{ | ||
| "microscope", | ||
| "test_tube", | ||
| "petri_dish", | ||
| "dna", | ||
| } |
There was a problem hiding this comment.
thought: While the emojis are a nice touch, I found them a little confusing. I spent some time trying to understand the meaning behind the emoji. I think it would be better to not include the emojis.
There was a problem hiding this comment.
@mwbrooks Oops! Returning to this now! It's confusing without obvious meaning... I agree! This was changed to use different emojis in da1d105 that separate "template" apps from "example" samples:
I'm curious what no emojis would look like though? I found this "column" ordering to be perhaps confusing in context with long lines. Perhaps we revisit app descriptions later? Or change these outputs elsewise?
There was a problem hiding this comment.
Thanks for taking the time to look back into this! Since the samples command has low usage, I think we can start by including the lengthy descriptions and revisit it later.
Co-authored-by: Michael Brooks <mbrooks@slack-corp.com>
@mwbrooks Thanks for reviewing and sharing suggestions alike! I made some updates in above commits and comments that reorder outputs to make things more clear I hope. I was hoping for a quick filter of current samples for similar outputs as the pages of docs.slack.dev/samples 🤖 Although I notice now we filter moreso by runtime without a "sample" and "template" option. I think this is alright to continue with but am open to all suggestion! Will rerequest a kind review! 🙏 ✨ |
mwbrooks
left a comment
mwbrooks
left a comment
There was a problem hiding this comment.
I'm curious what no emojis would look like though?
Personally, I'd lean toward no emojis. I think the nuances between examples and templates is difficult to understand. They're all sample apps and the more important filter is the framework language.
My two cents is to remove the emojis all together or use the same emoji for all samples.
✅ Tossing an approval on this one. Overall, it's moving the samples command in a better direction. Thanks @zimeg!
|
@mwbrooks Once again, thanks for the kind review 🙏 ✨
As a fast change to use one emoji I put together d2e22de but I'm hoping we can revisit outputs in future iteration 👾 Let's merge this now with follow up later! 🚢 |
Changelog
All available samples can be listed with the
slack samples --listcommand. The--languageflag might be found useful in filtering these outputs.Summary
This PR adds a
--listflag to thesamplescommand.Preview
📸
Reviewers
The following filters might be useful in experimentation! 🦠
Requirements