Contributor's Guide

From Dolphin Emulator Wiki
Jump to navigation Jump to search

Development workflow

HEAVILY WIP, DO NOT EXPECT THIS TO HELP YOU AT ALL AT THIS STAGE!

This page is meant to give you a short overview of how people usually develop their changes in combination with GitHub and how you can get your patches merged to the main repository.


General Development Workflow

Develop changes in local Git branches, publish them in GitHub forks. For quick overview on how to use Git and which remotes to use, read the Git introduction. Testing builds for most supported platforms can be created by buildbot-try. If there's a chance for regressions, make sure your changes get tested properly by others. Sometimes, it might be appropriate to create a testing request on the forums. If you think your branch is ready for merging, create a pull request on GitHub.

Handling Pull Request Comments

Once you have opened a pull request, people likely will have one or two little things to complain about. You should always these complaints serious (and yes, even things like the introduction of blank spaces at the end of lines matter). For comments you agree to, put one commit on top of your branch (one individual commit for each comment!). If you disagree or don't understand the comment's point, discuss why you disagree or ask for clarification, and if applicable wait for other people's opinions. If any arguments are made on IRC, summarize the result of them in a comment in the pull request.

Do not feel forced to agree with all reviewer remarks:

  • Sometimes the reviewer is wrong
  • Sometimes the reviewer is missing some context - maybe adding comments in the code would make sense
  • Sometimes the reviewer suggests something different that would be equally good. If it is a matter of preference only, feel free to ignore the proposed change.

If you feel like one of these scenarios applies to you, explain your concerns and we’ll decide how to continue. So far people have shown understanding in this kind of scenario. However, bear in mind that if you're just reluctant to spend the time to follow general rules (cf. the section on Quality Assurance), chances are your branch indeed will just get rejected - our desire for preserving code quality and stability is higher than having random feature XYZ, regardless of how awesome it might be.

Squashing

Once it seems like everybody is happy, clean up your pull request by squashing any commits that should be part of other ones:

git rebase -i master  # (or whatever commit your branch is based on)

Don't squash your whole pull request into one commit! Only squash incremental fixes to your previous commits and stuff like that. Also, avoid unnecessary rebasing/squashing when there is an active discussion on a commit, doing so causes GitHub to delete comments.

Quality Assurance

Pay attention to details when working on your changes! Some of the suggestions in this section might seem somewhat pedantic, but such things take virtually no time for you to consider once you are aware of them, but also eventually spare a lot of time and nerves for other people.

Git usage:

Most importantly, group changes in sensible, well-separated commits. This makes it easier to review them, and isolated changes are easier to debug if your branch later turns out to cause regressions. If the branch is already done and you already have one big commit covering everything, you should think about rewriting the branch from scratch, but with better separated commits (it's really worth the effort!). If you have a ridiculous amount of little patches, all of which are well-separated, do not come to the wrong conclusion that merging them is a good idea - it's not. We can afford bumping the commit count by a hundred, but we can't afford wasting time looking through a 1000 kB diff.

Keep a clean Git log: Use proper names for branches since they are shown in merge messages. In particular, work on a branch different than master unless it's a really generic change for which no fitting branch name can be given. Do not use dumb names like "neobrain-stuff", rather use something like "videosoftware-xfb-support" (all lowercase and separation via "-" preferred). Do not synchronize your local branches by merging origin/master into them. Instead, rebase the branch on origin/master regularly (regular rebasing will spare you a LOT of work and nerves for long-living branches). Reviewing a branch with more merge messages than actual commits is a confusing mess.

DO NOTs for commit messages:

  • No "Fix stupid mistake from previous revision". a) Explain what the issue was b) don't refer to "previous revision", mention the actual Git hash of the revision instead c) Actually, this commit shouldn't occur in the history at all - it should be squashed into "previous revision" instead.
  • No swearing or anything like that. We understand that development can be frustrating at times, particularly when you have to clean up other people's mistakes, but commit messages are not the place for that.
  • Do not write "Fix issue XYZ" without explaining what the issue was about or what you did to fix it. An issue tracker is not a replacement for a proper git log.

DOs for commit messages:

  • Use the first line of the commit message for a compact summary of the commit. This is because git's shortlog (and, in particular, the GitHub commit log viewer) only show the first line of a commit.
  • Use the following lines of the commit to provide further background, mention fixed issues from the issue tracker, ...
  • Use an efficient format for the messages, "$approximate_subsystem: $what\n\n$optionally_why", e.g. "PixelShaderGen: Changed calculations of alpha combiners to use integers.\n\nFixes an issue in Super Mario Sunshine where [...]"

Coding Style: Coding_Style

Getting Further Feedback

Come and join us on IRC! Our channel is #dolphin-dev on irc.libera.chat. This is generally a good idea, but it's particularly good to gather instant feedback on things. Who's_Who provides a list of active developers and their area of knowledge. If you're asking about something specific, it might be worth asking one of the listed developers directly. Note that some people, despite being shown as "online", might not actually look at IRC for various hours, so don't start raging when you don't get an instant reply after asking people directly.