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

[No QA] Continue merge without requiring editor #9775

Merged
merged 3 commits into from
Jul 8, 2022
Merged

Conversation

roryabraham
Copy link
Contributor

@roryabraham roryabraham commented Jul 7, 2022

Details

In the local testing of #9771 the editor was required to finish the merge commit. I thought that this wouldn't happen in the GH runner because they have pager disabled by default, but I was wrong. This PR makes it so that, even testing locally, the editor is not required to finish the merge commit.

Fixed Issues

$ n/a – broken deploys.

Local Tests

  1. Go to the App repo locally and run these commands:

    git fetch
    git checkout main
    git checkout -b fake-main
    git checkout main
    git checkout -b fake-staging
    echo "Update README on staging" >> README.md
    git add README.md
    git commit -m "Update README on staging"
    git checkout fake-main
    rm README.md
    git add README.md
    git commit -m "Delete README on main"
    git checkout fake-staging
    git merge -Xtheirs fake-main || {
      git diff --name-only --diff-filter=U | xargs git rm;  
      git merge -c core.editor=true --continue;
    }
  2. Verify that the merge in the last command succeeds and that it DOES NOT open an editor and prompt you for a commit message.

Live Tests

  1. Merge this PR
  2. RE-ship Deploy Checklist: New Expensify 2022-06-28 #9586 and the deploy should work.

@roryabraham roryabraham requested a review from a team as a code owner July 7, 2022 23:08
@roryabraham roryabraham self-assigned this Jul 7, 2022
@melvin-bot melvin-bot bot requested review from Luke9389 and removed request for a team July 7, 2022 23:09
Copy link
Contributor

@luacmartins luacmartins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and tests well!

@roryabraham roryabraham merged commit 6eb806a into main Jul 8, 2022
@roryabraham roryabraham deleted the Rory-FixDeploy branch July 8, 2022 00:13
@OSBotify
Copy link
Contributor

OSBotify commented Jul 8, 2022

🚀 Deployed to staging by @roryabraham in version: 1.1.81-0 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.82-5 🚀

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