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

[NO QA] Update the PR template to enforce issue linking formatting #4725

Merged
merged 4 commits into from
Aug 23, 2021

Conversation

mountiny
Copy link
Contributor

Details

We are automating a process to update issues solved by contributors so we can keep track of when to pay them and what the regression period is better. However, it works only if contributors link the PRs to the issues in a manner we expect. We are updating the PR template to make it more clear.

See the linked issue for more context.

Additionally, I am removing test.md file which I have introduced during testing webhook functionality for this issue https://github.com/Expensify/Expensify/issues/173928

Fixed Issues

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

Tests

No tests needed

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 August 18, 2021 09:56
@mountiny mountiny self-assigned this Aug 18, 2021
@MelvinBot MelvinBot requested review from aldo-expensify and removed request for a team August 18, 2021 09:56
@mountiny mountiny requested review from mallenexpensify and a team and removed request for aldo-expensify August 18, 2021 09:56
@MelvinBot MelvinBot requested review from aldo-expensify and removed request for a team August 18, 2021 09:56
@mountiny
Copy link
Contributor Author

@aldo-expensify Puller bear chose you, it is not specifically on Hold, but let's wait for @mallenexpensify to make sure the copy sounds right to him as well (we did not decide on exact form of the copy yet)

aldo-expensify
aldo-expensify previously approved these changes Aug 19, 2021
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
<!---
Please replace GH_LINK with the link to the GitHub issue this Pull Request is fixing.
Do NOT add the special GH keywords like `fixed` etc, we have our own process of managing the flow.
It MUST be an entire link to the issue, otherwise the linking will not work as expected.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sure, but I think usually there is a comma after 'otherwise'. Grammarly suggest putting it like:

It MUST be an entire link to the issue; otherwise, the linking will not work as expected.

I also found this sentence online that looks similar in structure: The sick woman needs to be given treatment; otherwise, her illness will become fatal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mallenexpensify Could we please get your 👀 on this copy. More eyes will know better.

Copy link
Contributor

Choose a reason for hiding this comment

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

That looks good to me 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect! Updated!

@aldo-expensify aldo-expensify self-requested a review August 19, 2021 00:45
@aldo-expensify aldo-expensify dismissed their stale review August 19, 2021 00:47

Approved by mistake

Co-authored-by: Aldo Canepa Garay <87341702+aldo-expensify@users.noreply.github.com>
@mountiny mountiny requested a review from a team as a code owner August 19, 2021 09:52
@MelvinBot MelvinBot requested review from yuwenmemon and removed request for a team August 19, 2021 09:52
@mountiny mountiny removed the request for review from yuwenmemon August 19, 2021 11:44
@mountiny
Copy link
Contributor Author

@yuwenmemon Sorry for the ping, that was some weird Pullerbear bug.

@mountiny
Copy link
Contributor Author

@aldo-expensify Thank you for looking over it, gonna wait for @mallenexpensify for his opinion. We should move forward with this along with other changes mentioned here.

@mallenexpensify
Copy link
Contributor

Looks good to me @mountiny, commented above too

@mountiny
Copy link
Contributor Author

@aldo-expensify I have updated it with your suggestions and it is ready for a final review 🎉

@mountiny
Copy link
Contributor Author

This PR is no longer on hold and can be reviewed/merged 🎉

Copy link
Contributor

@aldo-expensify aldo-expensify left a comment

Choose a reason for hiding this comment

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

Looks good!

@aldo-expensify
Copy link
Contributor

aldo-expensify commented Aug 20, 2021

I approved, but didn't merge in case @mallenexpensify wants to have a last look before.

@mallenexpensify
Copy link
Contributor

🚢 it!

@mountiny mountiny merged commit cad56eb into main Aug 23, 2021
@mountiny mountiny deleted the vit-updatePRTemplateAndRemoveDummyChange branch August 23, 2021 16: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.

@OSBotify
Copy link
Contributor

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

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