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

Add reopenIssueWithComment GitHub Action #2163

Merged
merged 35 commits into from
Apr 8, 2021

Conversation

deetergp
Copy link
Contributor

@deetergp deetergp commented Mar 30, 2021

PullerBear chooses: @Jag96

cc @roryabraham @AndrewGable

Details

This PR adds two GitHub Actions: checkDeployBlockers and reopenWithComment. The former evaluates recently closed issues, checking whether they have the StagingDeployCash label, and if they do, checks the issue description for open markdown checkboxes (- [ ]) and whether the last comment is missing the :shipit: string. If either is true, the latter action is run which reopens the issue and leaves a comment.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/155248

Tests

I mirrored all this logic in a personal test repo and verified that it worked. I did the following:

  1. Created an issue with three checklist items and left one unchecked.
  2. Closed the issue & verified that it was reopened with the expected comment.
  3. Checked the last box, close the issue, and verified that it reopened with the correct comment.
  4. Commented with the string :shipit: and close the issue gain
  5. Verified that the issue stayed closed.

Screen Shot 2021-04-05 at 5 21 30 PM

Unit Tests

tests/unit/checkDeployBlockersTest.js

@deetergp deetergp requested a review from a team as a code owner March 30, 2021 22:47
@deetergp deetergp self-assigned this Mar 30, 2021
@MelvinBot MelvinBot requested review from bondydaa and removed request for a team March 30, 2021 22:48
@deetergp deetergp removed the request for review from bondydaa April 1, 2021 21:56
@deetergp deetergp force-pushed the scott_reopenIssueWithComment branch from 3e25c14 to 08a400e Compare April 2, 2021 17:05
@deetergp deetergp requested a review from a team April 6, 2021 00:19
@MelvinBot MelvinBot requested review from Jag96 and removed request for a team April 6, 2021 00:19
@deetergp deetergp changed the title [WIP] Add reopenIssueWithComment GitHub Action Add reopenIssueWithComment GitHub Action Apr 6, 2021
@deetergp
Copy link
Contributor Author

deetergp commented Apr 6, 2021

Updated to address @roryabraham's comments.

roryabraham
roryabraham previously approved these changes Apr 6, 2021
Copy link
Contributor

@roryabraham roryabraham left a comment

Choose a reason for hiding this comment

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

Cool, I had a few organizational suggestions but all were pretty subjective so no blockers from me!

Copy link
Contributor

@Jag96 Jag96 left a comment

Choose a reason for hiding this comment

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

Looking good, just a few nabs/questions

.github/scripts/verifyActions.sh Outdated Show resolved Hide resolved
.github/actions/checkDeployBlockers/checkDeployBlockers.js Outdated Show resolved Hide resolved
Copy link
Contributor Author

@deetergp deetergp left a comment

Choose a reason for hiding this comment

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

I had to make a compromise in order to not kill all the tests with process.exit(0) nor get errors by return; -ing without a check.

@deetergp
Copy link
Contributor Author

deetergp commented Apr 7, 2021

Updated to address new comments.

roryabraham
roryabraham previously approved these changes Apr 7, 2021
Copy link
Contributor

@roryabraham roryabraham left a comment

Choose a reason for hiding this comment

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

Looking good!

@deetergp
Copy link
Contributor Author

deetergp commented Apr 7, 2021

Updated to address Joe's comment; added new test case for when there are no comments on an issue.

Copy link
Contributor

@Jag96 Jag96 left a comment

Choose a reason for hiding this comment

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

LGTM!

@roryabraham
Copy link
Contributor

Going to hold off on merging this today, because I want to run a production deploy and I don't want to introduce any new variables that might make it not work. This PR looks good, but I am 🐔 and don't want to test my luck until after we get all these PRs deployed.

@roryabraham roryabraham merged commit 308896d into master Apr 8, 2021
@roryabraham roryabraham deleted the scott_reopenIssueWithComment branch April 8, 2021 00:58
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