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.

Sign up for GitHub

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

Jump to bottom

🍒 Cherry pick PR #4020 to staging 🍒 #4033

Merged
merged 3 commits into from
Jul 14, 2021
Merged

🍒 Cherry pick PR #4020 to staging 🍒 #4033

merged 3 commits into from
Jul 14, 2021

Conversation

OSBotify
Copy link
Contributor

🍒 Cherry pick #4020 to staging 🍒

OSBotify and others added 2 commits July 14, 2021 12:30
@OSBotify
Merge pull request #4032 from Expensify/version-BUILD-0639c6771204ceb…
ef752cd 
…1c91926813fb2b3368c49a8b6

(cherry picked from commit 6cda0be)
@Beamanator
Merge pull request #4020 from Expensify/Rory-FixUserTypingIndicator
53fd87c 
Fix display of report typing indicator and participant local time

(cherry picked from commit 0639c67)
@OSBotify OSBotify requested a review from a team as a code owner July 14, 2021 12:30
@MelvinBot MelvinBot removed the request for review from a team July 14, 2021 12:30
@OSBotify
Copy link
Contributor Author

This pull request has merge conflicts and can not be automatically merged. 😞
Please manually resolve the conflicts, push your changes, and then request another reviewer to review and merge.
Important: There may be conflicts that GitHub is not able to detect, so please carefully review this pull request before approving.

@parasharrajat
Copy link
Member

@Beamanator Seems a conflict while merging

@Beamanator
Copy link
Contributor

Beamanator commented Jul 14, 2021 •
edited
Loading

Good catch @parasharrajat ! Hopefully the @Expensify/expensify-cash-deployers team can help with this, I haven't run into this before (related internal SO)

@AndrewGable
Copy link
Contributor

AndrewGable commented Jul 14, 2021 •
edited
Loading

You'll have to fix this merge conflict manually (and any others), then merge this PR to get this PR CP-ed: https://github.com/Expensify/Expensify.cash/pull/4033/files#diff-22037c1e4dbdea8803b450b3fce06e6c475762a31ecf84b34dc264ba0e443637R59-R64

@roryabraham
Copy link
Contributor

Ooooh spooky this hasn't happened yet.

@roryabraham
Copy link
Contributor

It seems in this case adding the Engineering label isn't enough to get this auto-assigned. I'll create an issue so we make sure issues like this are assigned to the author of the original CP PR

@roryabraham roryabraham self-assigned this Jul 14, 2021
@roryabraham
Copy link
Contributor

@Beamanator conflicts have been manually resolved and this is ready for review.

@roryabraham roryabraham mentioned this pull request Jul 14, 2021
Closed
23 tasks
@AndrewGable
Copy link
Contributor

@roryabraham - Are tests and lint applied to this PR?

@roryabraham
Copy link
Contributor

It seems they are not. I will make sure that's covered by the new issue as well.

@roryabraham
Copy link
Contributor

Issue here.

Beamanator
Beamanator approved these changes Jul 14, 2021
View reviewed changes
Copy link
Contributor

@Beamanator Beamanator left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good! 👍 Merge time (tested live-typing, font size & styles look great)

@roryabraham roryabraham merged commit 17e30ab into staging Jul 14, 2021
@roryabraham roryabraham deleted the cherry-pick-staging-4020 branch July 14, 2021 17:02
@OSBotify
Copy link
Contributor Author

🚀 Deployed to staging in version: 1.0.77-3🚀

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

@OSBotify
Copy link
Contributor Author

🚀 Deployed to production in version: 1.0.77-5🚀

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

@roryabraham
Copy link
Contributor

@parasharrajat looks like our CP deploy comment code is giving some false positives and commenting on the same PR twice

@parasharrajat
Copy link
Member

parasharrajat commented Jul 14, 2021 •
edited
Loading

So Do we not expect two comments for Staging and production deployment? Also Could you please explain further about the problem so that I can find problematic code?

@roryabraham
Copy link
Contributor

Oops, I commented on the wrong PR (I think because I was working from my mobile). Sorry about that, ignore me :oh_nothing:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Beamanator Beamanator Beamanator approved these changes

Assignees

@roryabraham roryabraham

Labels
automerge Engineering Hourly KSv2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@OSBotify @parasharrajat @Beamanator @AndrewGable @roryabraham