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

Fix the InternalQA checklist loosing its state #9458

Merged
merged 8 commits into from
Jun 16, 2022

Conversation

mountiny
Copy link
Contributor

@mountiny mountiny commented Jun 14, 2022

cc @yuwenmemon would you want to review, please? :)

Details

We were missing passing into the array of PRs those which have internalQA label, we did not parse the InternalQA section as we do for the deploy blockers.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/198446

Tests

  • Verify that no errors appear in the JS console

Automated tests already exist, they were false negatives previously showing this works although it did not work. Now they are true negatives as it should actually work.

@mountiny mountiny self-assigned this Jun 14, 2022
@mountiny mountiny requested a review from yuwenmemon June 15, 2022 16:47
@mountiny mountiny changed the title Fix the InternalQA checklist losing its state Fix the InternalQA checklist loosing its state Jun 15, 2022
@mountiny mountiny marked this pull request as ready for review June 15, 2022 16:49
@mountiny mountiny requested a review from a team as a code owner June 15, 2022 16:49
@melvin-bot melvin-bot bot requested review from PauloGasparSv and removed request for a team June 15, 2022 16:50
Copy link
Contributor

@yuwenmemon yuwenmemon left a comment

Choose a reason for hiding this comment

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

Nice find!

@yuwenmemon yuwenmemon merged commit 90c193a into main Jun 16, 2022
@yuwenmemon yuwenmemon deleted the vit-internalQAChecklist branch June 16, 2022 20:15
@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.

@roryabraham
Copy link
Contributor

@mountiny can you add some manual testing steps please? Maybe just:

  1. create a pull request
  2. label it internalQA
  3. Merge it with the checklist unlocked
  4. check it off on the QA checklist
  5. merge another PR with the checklist unlocked
  6. verify that the first PR is still checked

@roryabraham roryabraham added the InternalQA This pull request required internal QA label Jun 16, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jun 16, 2022

Triggered auto assignment to @yuwenmemon (InternalQA), see https://stackoverflow.com/c/expensify/questions/5042 for more details.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @yuwenmemon in version: 1.1.78-0 🚀

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

@mountiny
Copy link
Contributor Author

Good shout, thanks Rory!

@mountiny
Copy link
Contributor Author

Indeed something must be wrong with the regex, tested if the state of the checkbox has been kept and it was not: #9565

@mountiny
Copy link
Contributor Author

Good, I think I found what needs to be done to get this over the finish line.

@francoisl
Copy link
Contributor

@mountiny @yuwenmemon to confirm, there's no QA to do for this, right?

@mountiny
Copy link
Contributor Author

@francoisl the QA has failed unfortunately but Rory put up a fix. I am working on a follow up as the state of the checkboxes is still being lost, but this is fine for the deploy!

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @sketchydroide in version: 1.1.78-8 🚀

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
InternalQA This pull request required internal QA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants