Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NoQA] Fix deployBlocker workflow #4598

Merged
merged 1 commit into from
Aug 13, 2021

Conversation

francoisl
Copy link
Contributor

@francoisl francoisl commented Aug 12, 2021

@roryabraham please review cc @parasharrajat

This is a temporary fix until we find a better solution to escape any PR title.
This version would break with a PR that contains '" in its title.

https://github.com/Expensify/App/pull/4049/files
https://github.com/Expensify/App/pull/4186/files

Details

Fixed Issues

https://github.com/Expensify/App/runs/3307324034

https://expensify.slack.com/archives/C03V9A4TB/p1628733851287400

Tests

  1. Confirm you can actually reproduce the issue from the failed workflow. It failed when a deploy blocker was added to this PR.

    1. Create a small script.sh that contains the code from the failed workflow (GITHUB_ENV needs to be set, set anything as the value and it will output to that file)
GITHUB_ENV=output
echo "DEPLOY_BLOCKER_URL=https://github.com/Expensify/App/issues/4596" >> $GITHUB_ENV
echo "DEPLOY_BLOCKER_NUMBER=4596" >> $GITHUB_ENV
echo "DEPLOY_BLOCKER_TITLE=$(sed -e "s/'/'\\\\''/g; s/\`/\\\\\`/g; 1s/^/'/; \$s/\$/'/" <<< "'mWeb - Text is overlapping with the place holder text in What's for field'")" >> $GITHUB_ENV
$ bash -e script.sh
script.sh: line 4: unexpected EOF while looking for matching `''
  1. Wrap the PR title with "' ... '", try again
GITHUB_ENV=output
echo "DEPLOY_BLOCKER_URL=https://github.com/Expensify/App/issues/4596" >> $GITHUB_ENV
echo "DEPLOY_BLOCKER_NUMBER=4596" >> $GITHUB_ENV
echo "DEPLOY_BLOCKER_TITLE=$(sed -e "s/'/'\\\\''/g; s/\`/\\\\\`/g; 1s/^/'/; \$s/\$/'/" <<< "'mWeb - Text is overlapping with the place holder text in What's for field'")" >> $GITHUB_ENV
$ bash -e script.sh
$ cat output
DEPLOY_BLOCKER_URL=https://github.com/Expensify/App/issues/4596
DEPLOY_BLOCKER_NUMBER=4596
DEPLOY_BLOCKER_TITLE=''\''mWeb - Text is overlapping with the place holder text in What'\''s for field'\'''

Web

N/A

Mobile Web

N/A

Desktop

N/A

iOS

N/A

Android

N/A

This is a temporary fix until we find a better solution to escape any PR title.
This version would break with a PR that contains `'"` in its title.
@francoisl francoisl requested a review from roryabraham August 12, 2021 03:06
@francoisl francoisl requested a review from a team as a code owner August 12, 2021 03:06
@francoisl francoisl self-assigned this Aug 12, 2021
@MelvinBot MelvinBot requested review from MonilBhavsar and removed request for a team August 12, 2021 03:06
@roryabraham
Copy link
Contributor

This version would break with a PR that contains '" in its title.

Well, let's hope nobody includes that in a PR title. That would just be weird.

@roryabraham roryabraham merged commit 255bd5f into main Aug 13, 2021
@roryabraham roryabraham deleted the francois-fixDeployBlockerTitleParsing branch August 13, 2021 00:20
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @roryabraham in version: 1.0.85-10 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.0.86-11 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants