[NO QA] fix: action failure on invalid String in BASH #4049
Conversation
| @@ -54,7 +54,7 @@ jobs: | |||
| channel: '#deployer', | |||
| attachments: [{ | |||
| color: "#DB4545", | |||
| text: '💥 New E.cash Deploy Blocker: <${{ env.DEPLOY_BLOCKER_URL }}|${{ env.DEPLOY_BLOCKER_TITLE }}>', | |||
| text: '💥 New E.cash Deploy Blocker: <${{ env.DEPLOY_BLOCKER_URL }}|'+ `${{ env.DEPLOY_BLOCKER_TITLE }}`.replace(/(^'|'$)/gi, '').replace(/'\''/gi,'\'') + '>', | |||
There was a problem hiding this comment.
Hmmm I don't think this is likely to work? This isn't a JS string, it's using the expression syntax for GH actions.
There was a problem hiding this comment.
Look at here https://action-slack.netlify.app/with#custom_payload example. We can use js.
I checked the action code its is using eval();
There was a problem hiding this comment.
We can use js
Whoa, that's a nice feature of action-slack.
There was a problem hiding this comment.
Yeah totally agree, despite the use of eval.
There was a problem hiding this comment.
Can you also show me what local testing you did for this?
There was a problem hiding this comment.
This one is untested apart from the js code which I tested so that it gives us the original string.
I am not too sure about the + usages here. It could be the problem. But I feel it will work based on these:
- I tested Yaml syntax which is valid.
- Github will pass the string as it is after replacing the placeholders.
- Now eval should parse the value. So I tested if the string is valid js syntax. Which it is after Gh action placeholders replacements.
There was a problem hiding this comment.
Can you show me the tests you did for the JS replacement portion? Even just using a Node.js REPL and some sample strings to demonstrate how the replacement works.
There was a problem hiding this comment.
- Used this for YAML validation.
- for the eval part you can use this https://jsbin.com/roxenoyibu/edit?js,console to test out strings.
- You can use [NO QA] fix: action failure on invalid String in BASH #4049 (comment) to get strings.
|
Don't we also have to escape double-quotes? |
|
@tgolen All yours. Thanks |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging in version: 1.0.79-5🚀
|
|
🚀 Deployed to production in version: 1.0.80-2🚀
|
Please review @roryabraham
Details
#3600 (comment)
Fixed Issues
$ #3600
Need to be tested live.