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

[HOLD Web-Expensify #31129] PR template updated to work well with AwaitingPayment functionality #3619

Merged
merged 2 commits into from
Jun 24, 2021

Conversation

mountiny
Copy link
Contributor

Details

This PR is updating a pull request template to reflect needs of new webhook functionality introduced in Expensify/Web-Expensify#31129. We need to avoid using native keywords such as fixes for the issues so we do not close the external issues upon merging the PR, but we keep them open for the 7 days regression period.

cc @roryabraham

Fixed Issues

Tests

No tests

QA Steps

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@mountiny mountiny requested a review from a team as a code owner June 16, 2021 17:39
@mountiny mountiny self-assigned this Jun 16, 2021
@MelvinBot MelvinBot requested review from Beamanator and removed request for a team June 16, 2021 17:39
Beamanator
Beamanator previously approved these changes Jun 17, 2021
Copy link
Contributor

@Beamanator Beamanator left a comment

Choose a reason for hiding this comment

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

I like 👍

Are you sure we have to wait for Web-E#31129 to be merged before this one? If we merge this now, the only change is that issues won't be auto-closed when PRs are merged, right? I don't think that's a huge problem for now :D But I'm definitely happy to wait for the other PR to get merged first 👍

@mountiny
Copy link
Contributor Author

Yes, you are right! I had this discussion about merging or not merging here with Rafe. I would wait closer to the time the other PR will be merged as it might take couple more days and then it will also take some time before it gets to production. So I would wait for that yet.

@roryabraham
Copy link
Contributor

I know we talked about this already, but I later realized that other repos (such as Web-E) use $ as a special character to mark linked issues instead of Fixes. Maybe we want to follow that same pattern here for consistency?

@mountiny
Copy link
Contributor Author

Updated!

@mountiny mountiny requested a review from Beamanator June 18, 2021 15:31
@Beamanator Beamanator merged commit 62f06d8 into main Jun 24, 2021
@Beamanator Beamanator deleted the vit-updatedPullRequestTemplate branch June 24, 2021 06:56
@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.

@Beamanator
Copy link
Contributor

Oof I got merge-happy, and remembered @mountiny recommended we don't merge until Web-Expensify PR#31129 is merged / closer to being merged! Shall we revert?

@mountiny
Copy link
Contributor Author

That is a good question. I do not think it is necessary to revert but now for all the PRs created using this template the linked issues wont be automatically closed, so maybe we should let someone know.

I will try to push the other PR since it should be ready for a review!

@Beamanator
Copy link
Contributor

Beamanator commented Jun 24, 2021

I'll mention it in a slack channel or two :D

@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.73-4🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.74-0🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 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.

4 participants