tanzu-framework icon indicating copy to clipboard operation
tanzu-framework copied to clipboard

animals as text instead of emoji

Open jayunit100 opened this issue 2 years ago • 11 comments
trafficstars

fixes https://github.com/vmware-tanzu/tanzu-framework/issues/4183 old output:

15T02:32:53Z [ℹ]  🦊 - building plugin at path "cmd/cli/plugin/package"
15T02:32:53Z [ℹ]  🐰$ /usr/local/go/bin/go mod download
...

new output:

2022-12-15T14:42:11-05:00 [ℹ]  chicken - building plugin at path "cmd/cli/plugin-admin/test"
2022-12-15T14:42:11-05:00 [ℹ]  sheep   - building plugin at path "cmd/cli/plugin-admin/builder"
2022-12-15T14:42:11-05:00 [ℹ]  pony    - building plugin at path "cmd/cli/plugin-admin/codegen"
2022-12-15T14:42:11-05:00 [ℹ]  chicken$ /usr/local/go/bin/go mod download
2022-12-15T14:42:11-05:00 [ℹ]  pony   $ /usr/local/go/bin/go mod download
2022-12-15T14:42:11-05:00 [ℹ]  sheep  $ /usr/local/go/bin/go mod download

jayunit100 avatar Dec 15 '22 19:12 jayunit100

Codecov Report

Merging #4184 (ac28cc4) into main (6cdbd30) will decrease coverage by 0.90%. The diff coverage is n/a.

:exclamation: Current head ac28cc4 differs from pull request most recent head fb1fb1b. Consider uploading reports for the commit fb1fb1b to get more accurate results

@@            Coverage Diff             @@
##             main    #4184      +/-   ##
==========================================
- Coverage   49.55%   48.65%   -0.90%     
==========================================
  Files         452      482      +30     
  Lines       44849    46969    +2120     
==========================================
+ Hits        22225    22853     +628     
- Misses      20513    21954    +1441     
- Partials     2111     2162      +51     
Impacted Files Coverage Δ
cmd/cli/plugin/cluster/get_machinehealthcheck.go 11.42% <0.00%> (ø)
cmd/cli/plugin/isolated-cluster/main.go 0.00% <0.00%> (ø)
cmd/cli/plugin/cluster/get_node_pools.go 10.52% <0.00%> (ø)
.../isolated-cluster/imgpkginterface/client_imgpkg.go 0.00% <0.00%> (ø)
...in/cluster/set_machinehealthcheck_control_plane.go 21.21% <0.00%> (ø)
...lugin/isolated-cluster/fakes/client_imgpkg_fake.go 32.75% <0.00%> (ø)
cmd/cli/plugin/cluster/main.go 0.00% <0.00%> (ø)
...cluster/delete_machinehealthcheck_control_plane.go 16.66% <0.00%> (ø)
cmd/cli/plugin/cluster/machinehealthcheck.go 100.00% <0.00%> (ø)
cmd/cli/plugin/isolated-cluster/test/main.go 0.00% <0.00%> (ø)
... and 21 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Dec 15 '22 20:12 codecov[bot]

If these are truly just concurrency identifiers, can't we just use "Thread 01-Thread 12", or "Job 01-Job 12" (or with zero-based numbering if you prefer)?

jmoroski avatar Dec 15 '22 20:12 jmoroski

i like the animal names but if its a "hard no" then ill replace w/ ints let me know @jmoroski .

In all seriousness, here is my case for keeping the "animal spirit" alive:

  • they are easier to read /search / reference then arbitrary integers.
  • the intent of the emoji's in the original framework, to have a little creative expression in an otherwise mundane build system, are IMO worth preserving.

either way is fine tho :)

just lmk if you'd strongly prefer int IDs, ill do:

T1
T2
T3
T4
...

so theyr unique and searchable...

jayunit100 avatar Dec 15 '22 21:12 jayunit100

... but if its a "hard no" then ill replace w/ ints

If this was internal only, I'd tell you to use whatever tasteful collection of animals you choose.

But I believe this builder plugin is what we currently tell people outside of our team (and possibly organization) to use to scaffold and build plugins. So IDs is the less fun, but better choice. I'm fine with your T1, T2, ...Tn option.

jmoroski avatar Dec 15 '22 23:12 jmoroski

Ack yup t1 is traceable no problem will fix it to that

jayunit100 avatar Dec 17 '22 03:12 jayunit100

fixed the emjois to T1-9 !

jayunit100 avatar Dec 21 '22 18:12 jayunit100

LGTM

knabben avatar Dec 21 '22 18:12 knabben

Thanks @jayunit100 . Looking specifically at the output I think we have some inconsistencies. This is what I see:

2022-12-21T15:52:51-05:00 [ℹ]  building local repository at artifacts/darwin/arm64/cli
2022-12-21T15:52:51-05:00 [ℹ]  	starting compile thread 0 for &{cmd/cli/plugin cluster 2147483648 <nil>}
2022-12-21T15:52:51-05:00 [ℹ]  	starting compile thread 1 for &{cmd/cli/plugin feature 2147483648 <nil>}
2022-12-21T15:52:51-05:00 [ℹ]  	starting compile thread 2 for &{cmd/cli/plugin isolated-cluster 2147483648 <nil>}
2022-12-21T15:52:51-05:00 [ℹ]  	starting compile thread 3 for &{cmd/cli/plugin login 2147483648 <nil>}
2022-12-21T15:52:51-05:00 [ℹ]  	starting compile thread 4 for &{cmd/cli/plugin managementcluster 2147483648 <nil>}
2022-12-21T15:52:51-05:00 [ℹ]  	starting compile thread 5 for &{cmd/cli/plugin package 2147483648 <nil>}
2022-12-21T15:52:51-05:00 [ℹ]  	starting compile thread 6 for &{cmd/cli/plugin pinniped-auth 2147483648 <nil>}
2022-12-21T15:52:51-05:00 [ℹ]  	starting compile thread 7 for &{cmd/cli/plugin secret 2147483648 <nil>}
2022-12-21T15:52:51-05:00 [ℹ]  	starting compile thread 8 for &{cmd/cli/plugin telemetry 2147483648 <nil>}
2022-12-21T15:52:51-05:00 [ℹ]  T6 - building plugin at path "cmd/cli/plugin/login"
2022-12-21T15:52:51-05:00 [ℹ]  T3 - building plugin at path "cmd/cli/plugin/cluster"
2022-12-21T15:52:51-05:00 [ℹ]  T8 - building plugin at path "cmd/cli/plugin/package"
2022-12-21T15:52:51-05:00 [ℹ]  T10 - building plugin at path "cmd/cli/plugin/secret"
2022-12-21T15:52:51-05:00 [ℹ]  T7 - building plugin at path "cmd/cli/plugin/managementcluster"
2022-12-21T15:52:51-05:00 [ℹ]  T9 - building plugin at path "cmd/cli/plugin/pinniped-auth"
2022-12-21T15:52:51-05:00 [ℹ]  T8$ /Users/kmarc/.asdf/shims/go mod download
2022-12-21T15:52:51-05:00 [ℹ]  T6$ /Users/kmarc/.asdf/shims/go mod download
2022-12-21T15:52:51-05:00 [ℹ]  T5 - building plugin at path "cmd/cli/plugin/isolated-cluster"
2022-12-21T15:52:51-05:00 [ℹ]  T9$ /Users/kmarc/.asdf/shims/go mod download
2022-12-21T15:52:51-05:00 [ℹ]  T5$ /Users/kmarc/.asdf/shims/go mod download
2022-12-21T15:52:51-05:00 [ℹ]  T4 - building plugin at path "cmd/cli/plugin/feature"
2022-12-21T15:52:51-05:00 [ℹ]  T7$ /Users/kmarc/.asdf/shims/go mod download
2022-12-21T15:52:51-05:00 [ℹ]  T10$ /Users/kmarc/.asdf/shims/go mod download
2022-12-21T15:52:51-05:00 [ℹ]  T4$ /Users/kmarc/.asdf/shims/go mod download
2022-12-21T15:52:51-05:00 [ℹ]  T3$ /Users/kmarc/.asdf/shims/go mod download

I realize this is nit-picky, but since we have our hands in there, might as well...

  1. Since T1 seems to mean thread 1 and right above we have a (new from last week) printout saying starting compile thread X, I find it a bit confusing the the two number don't match for the plugin being compiled.
  2. I also find that the output that includes $ looks surprising now that it is not an emoji preceding it. For example T8$ make it look like the $ is part of the ID. How about replacing the $ with - like for the other lines?
  3. I like @jmoroski suggestion of padding with a 0 to get some nicer alignment in the above.
  4. I believe the [i] which follows the timestamp will also not show properly for dumb terminals. Not much we can do about that at the moment though as it comes from the logging library.
  5. Not related to your change, but this new printout starting compile thread 1 for &{cmd/cli/plugin feature 2147483648 <nil>} does not have a very nice second part, does it? Should we fix it?

Just some suggestions...

marckhouzam avatar Dec 21 '22 21:12 marckhouzam

Now im starting to think Maybe I should role this in w a broader klog replacement :)

jayunit100 avatar Dec 22 '22 00:12 jayunit100

I echo @marckhouzam 's thoughts. The padding for better alignment, using matching ids for the "starting.." log line, as well as tweaking the $ looks straightforward enough to add.

vuil avatar Dec 22 '22 00:12 vuil

just saw this, fixed mark's suggestions... but im also going to look into klog more broadly as well today.

jayunit100 avatar Jan 03 '23 16:01 jayunit100