removes arbitrary whitespace before unicode char/emojis
there's inconsistent usage of whitespace before and after emoji usage in log messages making the output excessively messy and messages misaligned.
this a bandaid fix as I could prob refactor the logger to use a standard convention so whitespace isn't hardcoded but for the time being this would be a welcome partial fix
Before fix output example:
After fix output example:
I thought this random indentation is intentional, but this is already very old code before I changed anything here. I don't think this is an area I would decide any direction.
I'm not an fan of the emoji usage in act, but I might should have helped panekj more in his PR to remove the emojis a looooong time ago.
This PR adds two additional emojis, not a fan of this
whether to leave them in or remove them can we agree that it's currently inconsistent throughout the code base and should have something done. at the very least can we get this fix in so the beginning of messages are atleast more aligned then they currently are?
I'm fine with merging this if someone is bothered by the space count. For reference this is the PR that was supposed to fix logging: https://github.com/nektos/act/pull/928
I thought this random indentation is intentional
i also thought maybe there was some form of intentionality to it but i haven't been able to decipher anything, seems random. Maybe just my poor reading comprehension but it's more distracting than anything imo
This is ok for me, but I only approve if necessary for merging once the PR gets one counting approval from someone else.
Or if emoji up vote of 10+ happens for the original post, I cannot merge either way on my own.
FYI changing these tests to pass is required as well
✖ pkg/runner (5m28.277s) (coverage: 57.0% of statements in ./...)
=== Failed
=== FAIL: pkg/runner TestAddmaskUsemask (0.00s)
command_test.go:174:
Error Trace: /home/runner/work/act/act/pkg/runner/command_test.go:174
Error: Not equal:
expected: "[testjob]⚙ ***\n[testjob]⚙ ::set-output:: = token=***\n"
actual : "[testjob] ⚙ ***\n[testjob] ⚙ ::set-output:: = token=***\n"
Diff:
--- Expected
+++ Actual
@@ -1,3 +1,3 @@
-[testjob]⚙ ***
-[testjob]⚙ ::set-output:: = token=***
+[testjob] ⚙ ***
+[testjob] ⚙ ::set-output:: = token=***
Test: TestAddmaskUsemask
DONE 1153 tests, 2 failures in 379.272s
exit status 1
Please Ignore the TestImageExistsLocally Test, that might only need a rerun
@ChristopherHX this should be fixed now, looks like it was just a consequence of 3x space being used in the test string.
For future ref; what's the stance on using either a ~/.config/act/config.toml (or whatever file format) or looking for a environment variable (e.g. ACT_PLAIN_OUTPUT=true) to disable emojis? This would dictate what a purposed (better?) solution would look like for a refactor of logger to handle emojis
Codecov Report
Attention: Patch coverage is 89.65517% with 3 lines in your changes missing coverage. Please review.
Project coverage is 74.41%. Comparing base (
5a80a04) to head (0810d44). Report is 187 commits behind head on master.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| pkg/runner/command.go | 92.85% | 1 Missing :warning: |
| pkg/runner/run_context.go | 75.00% | 1 Missing :warning: |
| pkg/runner/step.go | 80.00% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #2680 +/- ##
===========================================
+ Coverage 61.56% 74.41% +12.85%
===========================================
Files 53 72 +19
Lines 9002 11062 +2060
===========================================
+ Hits 5542 8232 +2690
+ Misses 3020 2193 -827
- Partials 440 637 +197
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
🚀 New features to boost your workflow:
- ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.