Conversation
The output is otherwise very easy to miss. changelog: Internal, Makefile, events ordering linter outputs errors in bold text
| def account_delete_visited | ||
| track_event('Account Delete visited') | ||
| end | ||
|
|
There was a problem hiding this comment.
This is moved only to demo the error, and will be undone before merge!
Makefile
Outdated
| lint_analytics_events_sorted: | ||
| @test "$(shell grep '^ def ' app/services/analytics_events.rb)" = "$(shell grep '^ def ' app/services/analytics_events.rb | sort)" \ | ||
| || (echo 'Error: methods in analytics_events.rb are not sorted alphabetically' && exit 1) | ||
| || (echo -e '\033[1;31mError: methods in analytics_events.rb are not sorted alphabetically\033[0m' && exit 1) |
There was a problem hiding this comment.
The -e here appears to be output literally in the build output?
There was a problem hiding this comment.
But StackOverflow said to use it! 
;)
I had noticed this locally, and chalked it up to a Mac vs. coreutils issue or something. I wonder if it's actually bash vs. zsh and it's a built-in?
Since it seems extraneous either way I'll remove it!
Are you generally on board with the idea of this PR?
There was a problem hiding this comment.
Part of the reason I thought to look is my own history with common commands having different support across OS, so I suspected maybe that's why it wasn't working as expected but could be something that a StackOverflow describes.
Are you generally on board with the idea of this PR?
I think it makes sense if it helps boost visibility of the issue. My main concerns would be (a) applying this ad-hoc rather than across the board, and (b) the color codes hurt the readability of the code. But overall I feel it could be a net-positive 👍
There was a problem hiding this comment.
My main concerns would be (a) applying this ad-hoc rather than across the board, and (b) the color codes hurt the readability of the code. But overall I feel it could be a net-positive 👍
Thanks! I am trying to keep the scope on this as small as I can because I don't have a ton of spare cycles, but I agree with both of these. I was tempted to implement a little echo_red function or something, but it felt a little weird to put that in a Makefile. And making the other echos for errors red should really result in me testing triggering them.
I'll take a screenshot of this without the -e, then back out the change to AnalyticsEvents that was introduced to trigger this, and then merge. If someone is feeling inspired down the road we can extend it.
|
With the
|
2 participants
🎫 Ticket
None. I got annoyed and did this.
🛠 Summary of changes
If this Makefile-implemented linter fails, the error is now in bold red text.
Why? Because it took me way too long to figure out why this run failed when there were no apparent errors.
Notes