Simplified pull request template #21900

vkjr · 2025-01-08T18:50:47Z

Summary

Pull request template edited to remove outdated info and unify comments.

Review notes

You can use edit option on this section to see how template looks.

Testing notes

Platforms

Android
iOS

Areas that maybe impacted

Functional

1-1 chats
public chats
group chats
wallet / transactions
dapps / app browsing
account recovery
new account
user profile updates
networks
mailservers
fleet
bootnodes

Non-functional

battery performance
CPU performance / speed of the app
network consumption

Steps to test

Open Status
...
Step 3, etc.

status: ready

vkjr · 2025-01-08T18:51:27Z

.github/PULL_REQUEST_TEMPLATE.md

 [comment]: # (To auto-close issue on merge, please insert the related issue number after # i.e fixes #566)
-
-If you submit PR for issue with bounty then write here Fixes #NN where NN is issue number


We don't have bounties now

We actually have means of compensating work from external contribution. This has been done during 2024 and there are at least two developers that I know interested in that.

But I think we should keep the removal as you did. This is unnecessary noise.

@ilmotta is there a public way to apply as an external contributor? I know a few that'd be interested on it

Let's talk in private about this @ulisesmac, there are some important details first. If anybody is interested to understand this point, please reach out to me.

vkjr · 2025-01-08T18:52:09Z

.github/PULL_REQUEST_TEMPLATE.md

 [comment]: # (Summarise the problem and how the pull request solves it)
 ...

-<!-- (Optional, remove if no changes to documentation) -->
-Documentation change PR (review please): https://github.com/status-im/status.im/pull/xxx


We do not change documentation frequently

vkjr · 2025-01-08T18:52:36Z

.github/PULL_REQUEST_TEMPLATE.md


 - Android
 - iOS
- macOS


desktop platforms aren't actual in status-mobile repo

I think this is related to changes in dev exp.

But yeah, they aren't common, let's remove it

@ulisesmac, I think that was added back in times when it was possible to build desktop version of status-mobile using react-native-desktop-qt. That was long ago)

vkjr · 2025-01-08T18:53:27Z

.github/PULL_REQUEST_TEMPLATE.md


 - Open Status
 - ...
 - Step 3, etc.

-<!-- (PRs will only be accepted if squashed into single commit.) -->


This is enforced by github machinery.

Indeed, this has confused a bunch of people over time. The only thing I'm not sure is that some devs may still prefer to merge via command line, in which case I'm not sure GitHub will reject if the commits aren't squashed. In status-go GitHub happily accepts multiple commits because I've seen this happen a few times.

@ilmotta, well, if developer going to use command line, pull request comment won't stop it from happening.

vkjr · 2025-01-08T18:54:15Z

.github/PULL_REQUEST_TEMPLATE.md


 - Open Status
 - ...
 - Step 3, etc.

-<!-- (PRs will only be accepted if squashed into single commit.) -->
-
-### Before and after screenshots comparison


Never seen this section used. Usually screenshots are attached in summary

This was used a couple times, for example I'm pretty sure I've seen in PRs for quo components. But fair enough to remove 👍🏼 I have a macro to generate an HTML table for before/after screenshots. At one point in Status I used this macro a lot, but never this part from the template.

status-im-auto · 2025-01-08T18:56:14Z

Jenkins Builds

Click to see older builds (4)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`d3e127a`	#1	2025-01-08 18:56:13	~5 min	`tests`	📄`log`
✔️	`d3e127a`	#1	2025-01-08 18:58:24	~7 min	`ios`	📱`ipa` 📲
✔️	`d3e127a`	#1	2025-01-08 19:00:14	~9 min	`android-e2e`	🤖`apk` 📲
✔️	`d3e127a`	#1	2025-01-08 19:00:45	~9 min	`android`	🤖`apk` 📲

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`d0e4d24`	#3	2025-01-09 12:45:55	~4 min	`tests`	📄`log`
✔️	`d0e4d24`	#3	2025-01-09 12:48:18	~6 min	`android`	🤖`apk` 📲
✔️	`d0e4d24`	#3	2025-01-09 12:49:00	~7 min	`ios`	📱`ipa` 📲
✔️	`d0e4d24`	#3	2025-01-09 12:49:03	~7 min	`android-e2e`	🤖`apk` 📲

✔️	`b8111db`	#4	2025-01-09 13:04:21	~4 min	`tests`	📄`log`
✔️	`b8111db`	#4	2025-01-09 13:06:20	~6 min	`android-e2e`	🤖`apk` 📲
✔️	`b8111db`	#4	2025-01-09 13:06:31	~6 min	`ios`	📱`ipa` 📲
✔️	`b8111db`	#4	2025-01-09 13:07:49	~8 min	`android`	🤖`apk` 📲

ilmotta · 2025-01-08T20:40:41Z

.github/PULL_REQUEST_TEMPLATE.md

 [comment]: # (To auto-close issue on merge, please insert the related issue number after # i.e fixes #566)
-
-If you submit PR for issue with bounty then write here Fixes #NN where NN is issue number


We actually have means of compensating work from external contribution. This has been done during 2024 and there are at least two developers that I know interested in that.

But I think we should keep the removal as you did. This is unnecessary noise.

ilmotta · 2025-01-08T20:42:29Z

.github/PULL_REQUEST_TEMPLATE.md


 #### Areas that maybe impacted
-<!-- (Optional. Specify if some specific areas has to be tested, for example 1-1 chats) -->
+[comment]: # (Optional. Specify if some specific areas has to be tested, for example 1-1 chats)


Suggestion: Specify if specific areas need to be tested, such as 1-1 chats.`

ilmotta · 2025-01-08T20:43:04Z

.github/PULL_REQUEST_TEMPLATE.md


 - Android
 - iOS
- macOS
- Linux
- Windows

 #### Areas that maybe impacted


maybe -> may be

ilmotta · 2025-01-08T20:47:28Z

.github/PULL_REQUEST_TEMPLATE.md


 - Open Status
 - ...
 - Step 3, etc.

-<!-- (PRs will only be accepted if squashed into single commit.) -->


Indeed, this has confused a bunch of people over time. The only thing I'm not sure is that some devs may still prefer to merge via command line, in which case I'm not sure GitHub will reject if the commits aren't squashed. In status-go GitHub happily accepts multiple commits because I've seen this happen a few times.

ilmotta · 2025-01-08T20:49:51Z

.github/PULL_REQUEST_TEMPLATE.md


 - Open Status
 - ...
 - Step 3, etc.

-<!-- (PRs will only be accepted if squashed into single commit.) -->
-
-### Before and after screenshots comparison


This was used a couple times, for example I'm pretty sure I've seen in PRs for quo components. But fair enough to remove 👍🏼 I have a macro to generate an HTML table for before/after screenshots. At one point in Status I used this macro a lot, but never this part from the template.

ilmotta · 2025-01-08T20:52:37Z

.github/PULL_REQUEST_TEMPLATE.md

 ### Review notes
-<!-- (Optional. Specify if something in particular should be looked at, or ignored, during review) -->
+[comment]: # (Optional. Specify if something in particular should be looked at, or ignored, during review)

 ### Testing notes


I usually raise the level on up. That makes the headline in GH have an underline and stand out better, also gives the person a chance to use multiple level-3 headlines if desired. Personal preference, but I'd vote for raising all headlines one level up.

@ilmotta do you mean having ## Review notes instead of ### Review notes?
upd: Applied that - I also like how it looks 👍

ulisesmac

Very much needed! 👍 Thanks @vkjr

vkjr requested review from ilmotta, briansztamfater, smohamedjavid, clauxx and ulisesmac January 8, 2025 18:50

vkjr self-assigned this Jan 8, 2025

vkjr requested a review from jakubgs as a code owner January 8, 2025 18:50

vkjr commented Jan 8, 2025

View reviewed changes

ilmotta reviewed Jan 8, 2025

View reviewed changes

ilmotta approved these changes Jan 8, 2025

View reviewed changes

ulisesmac approved these changes Jan 8, 2025

View reviewed changes

vkjr added the skip-manual-qa label Jan 9, 2025

vkjr force-pushed the update-pull-request-template branch from 92b2101 to d0e4d24 Compare January 9, 2025 12:41

vkjr added 2 commits January 9, 2025 12:59

Simplified pull request template

50f6174

review notes

b8111db

vkjr force-pushed the update-pull-request-template branch from d0e4d24 to b8111db Compare January 9, 2025 12:59

jakubgs approved these changes Jan 9, 2025

View reviewed changes

smohamedjavid approved these changes Jan 9, 2025

View reviewed changes

clauxx approved these changes Jan 9, 2025

View reviewed changes

vkjr merged commit 7709634 into develop Jan 9, 2025
5 checks passed

vkjr deleted the update-pull-request-template branch January 9, 2025 13:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplified pull request template #21900

Simplified pull request template #21900

vkjr commented Jan 8, 2025 •

edited

Loading

vkjr Jan 8, 2025

ilmotta Jan 8, 2025

ulisesmac Jan 8, 2025 •

edited

Loading

ilmotta Jan 8, 2025

vkjr Jan 8, 2025 •

edited

Loading

vkjr Jan 8, 2025

ulisesmac Jan 8, 2025

vkjr Jan 9, 2025

vkjr Jan 8, 2025

ilmotta Jan 8, 2025

vkjr Jan 9, 2025

vkjr Jan 8, 2025 •

edited

Loading

ilmotta Jan 8, 2025

status-im-auto commented Jan 8, 2025 •

edited

Loading

ilmotta Jan 8, 2025

ilmotta Jan 8, 2025

ilmotta Jan 8, 2025

ilmotta Jan 8, 2025

ilmotta Jan 8, 2025

ilmotta Jan 8, 2025

vkjr Jan 9, 2025 •

edited

Loading

ulisesmac left a comment

		[comment]: # (To auto-close issue on merge, please insert the related issue number after # i.e fixes #566)

		If you submit PR for issue with bounty then write here Fixes #NN where NN is issue number

Simplified pull request template #21900

Simplified pull request template #21900

Conversation

vkjr commented Jan 8, 2025 • edited Loading

Summary

Review notes

Testing notes

Platforms

Areas that maybe impacted

Functional

Non-functional

Steps to test

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulisesmac Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vkjr Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vkjr Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

status-im-auto commented Jan 8, 2025 • edited Loading

Jenkins Builds

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vkjr Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

ulisesmac left a comment

Choose a reason for hiding this comment

vkjr commented Jan 8, 2025 •

edited

Loading

ulisesmac Jan 8, 2025 •

edited

Loading

vkjr Jan 8, 2025 •

edited

Loading

vkjr Jan 8, 2025 •

edited

Loading

status-im-auto commented Jan 8, 2025 •

edited

Loading

vkjr Jan 9, 2025 •

edited

Loading