From 3dec4e4aabb93e511b0d6338b35784ea427c4550 Mon Sep 17 00:00:00 2001 From: GeckoEidechse <40122905+GeckoEidechse@users.noreply.github.com> Date: Thu, 4 May 2023 17:38:30 +0200 Subject: Add section about avoiding PR hell (#170) --- .../development/contributing-code-to-northstar.md | 48 ++++++++++++++++++++++ 1 file changed, 48 insertions(+) (limited to 'docs/modding-and-development') diff --git a/docs/modding-and-development/development/contributing-code-to-northstar.md b/docs/modding-and-development/development/contributing-code-to-northstar.md index 8633e83..98cf770 100644 --- a/docs/modding-and-development/development/contributing-code-to-northstar.md +++ b/docs/modding-and-development/development/contributing-code-to-northstar.md @@ -30,3 +30,51 @@ To get the cropped GIFs of a screenrecording: 3. Use a site like [https://ezgif.com/video-to-gif](https://ezgif.com/video-to-gif) for cropping and turning it into a GIF \ (The linked site has not been vetted, use at own risk) 4. Upload GIF to GitHub pull request description by just pasting it at the appropriate location. + +### Avoiding "PR hell" + +_"PR hell"_ is what contributors refer to when a pull request is stuck as pending without any indication for progress. + +There can be a variety reasons for this. Whether it's because the pull request was overlooked, the changes are too big that anyone actually dares looking at it, or simply every reviewer is too busy at the moment, not getting a response on something you put a lot of work into is a terrible experience. + +While we are continuously working on improving the experience and recruiting more reviewers, here's a few tips to avoid _"PR hell"_. + +**Make sure your PR is easy to review:** + +If you are submitting a new feature, highlight it with screenshots, screenrecordings, GIFs etc. If you can make your change look interesting, it highly increases the chance of someone becoming interested in getting it merged. + +If you are submitting a bug fix, make sure to mention the steps to reproduce the bug and that the same steps no longer cause the bug on your fixed version. + +Finally, smaller code changes are easier to review than bigger ones, which brings us to: + +**Keep PRs concise and focused on a specific issue:** + +While working on code to create some feature or fix some issue you might come across some other things you also want to change. While this is fine, it's highly recommended to split out these changes into separate PRs. + +Having lots of small PRs is way easier to review and work with than one big PR. Further if there's an issue with one of the changes it does not block the whole big PR but just one of the small ones. + +Smaller changes are also way easier to review and test. + +If you're working on a big feature that introduces lots of changes (let's say you implemented an entire party system into Northstar) a recommended approach is to PR your big change so that feature can be easily observed and interest generated. \ +Then ask which parts should be turned into separate pull requests, PR those, and link them with your big PR. As the the smaller PRs are merged, the diff of the big PR should shrink overtime until concise enough to be merged all at once. + +Just make sure to resolve merge conflicts when the smaller PRs are merged ;) + +**Ping previous author:** + +Especially when you fix a bug in existing code, you usually modify code that was written by someone else. In that case the best person to review that code is the original author. So `@` ping them in the pull request! + +You can use tools like [`git blame`](https://www.git-scm.com/docs/git-blame) (on the web via GitHub, in vscode via gitlens, via command-line, ...) to see who previously wrote the code. + + +**Ping for updates:** + +Sometimes it can happen that a pull request simply gets lost in the sea of other pull requests. If you haven't received a response in a while on a pull request that has already received some responses, just leave a comment on it (even if it's just "bump"). This will send a notification to everyone that responded previously in the PR and likely cause them to respond again. + +Similarly, you can also check merged pull requests and see who reviewed/merged them and ping that person individually. + +**If all else fails:** + +If despite all your attempts you are still not seeing responses, your last resort is always to ask around in `#development` on the Northstar Discord. We really do not want to see anyone become frustrated with the lack of responses on their pull request and close it in frustration. As such please reach out earlier rather than later. + +We will try our best to look at it <3 -- cgit v1.2.3