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

[No QA] Auto-check NO-QA PRs #6056

Merged
merged 4 commits into from
Oct 26, 2021
Merged

[No QA] Auto-check NO-QA PRs #6056

merged 4 commits into from
Oct 26, 2021

Conversation

roryabraham
Copy link
Contributor

Details

Automatically checks off any PRs with titles prefixed by [NO QA]

Fixed Issues

$ n/a – just a QOL improvement and part of testing #6047

Tests

  1. Merge this PR.
  2. It should be added to the checklist and already checked off.

QA Steps

None.

@roryabraham roryabraham self-assigned this Oct 25, 2021
@roryabraham roryabraham requested a review from a team as a code owner October 25, 2021 23:36
@MelvinBot MelvinBot requested review from timszot and removed request for a team October 25, 2021 23:36
stitesExpensify
stitesExpensify approved these changes Oct 26, 2021
View reviewed changes
.github/libs/GithubUtils.js
'html_url',
);
console.log('Found the following NO QA PRs:', noQAPRs);
const verifiedOrNoQAPRs = _.union(verifiedPRList, noQAPRs);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to double check, the intention here is that no qa means no QA or accessibility testing right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that's the intention!

timszot
timszot reviewed Oct 26, 2021
View reviewed changes
Copy link
Contributor

@timszot timszot left a comment

Choose a reason for hiding this comment

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

LGTM

NAB: There's a lot of duplication of code here. Can it be broken out to a utility function somewhere for all these files to use?

@roryabraham
Copy link
Contributor Author

There's a lot of duplication of code here. Can it be broken out to a utility function somewhere for all these files to use?

@timszot All the index.js files are compiled files, see Why do actions need to be compiled?

@roryabraham roryabraham merged commit 9632938 into main Oct 26, 2021
@roryabraham roryabraham deleted the Rory-AutoCheckOffNoQA branch October 26, 2021 16:03
@OSBotify OSBotify mentioned this pull request Oct 26, 2021
Closed
@roryabraham
Copy link
Contributor Author

Nice, this passed QA 🎉

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @roryabraham in version: 1.1.9-3 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.10-2 🚀

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
Reviewers

@timszot timszot timszot left review comments

@stitesExpensify stitesExpensify stitesExpensify approved these changes

Assignees

@roryabraham roryabraham

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@roryabraham @OSBotify @timszot @stitesExpensify