How to send me a pull request

I find myself repeating a lot of the same comments in pull requests, so I decided to put together a list of what I consider the most important features of a good pull request. Other people will have different feelings on some of these, but the points below are what apply to my projects. If you have thoughts on things I've left out, or reasons why you disagree with these points, please comment below.

Many of these points only make sense for source code, and even more specifically for code written in Haskell. My content repos (like this site's content) and non-Haskell repos (do I have any of those???) would be slightly different.

NOTE: I'm not the maintainer of Stack, so the comments below do not necessarily apply there. Stack has its own contribution rules, so please don't take my personal opinions here as relevant to that project.

• Every top-level identifier exported from a library needs to have a Haddock comment. It's irrelevant if the identifier name is completely self-commenting; a comment is still necessary. (These are the comments that look like -- | Describe what the function does.)

• My packages all follow PVP-style version numbers, and pull requests should include bumps to the version number. For those unfamiliar: PVP version numbers consistent of four components, A.B.C.D.

  • If your change fixes a bug without modifying the API at all, then bump D (1.2.3.4 becomes 1.2.3.5, or 1.2.3 becomes 1.2.3.1).

  • If your change adds something new to the API without changing something that already exists, bump C (1.2.3.4 becomes 1.2.4).

  • If you change the existing API (e.g., remove a function, change semantics, modify a data type), bump either A or B (1.2.3.4 becomes either 1.3.0 or 2.0.0, depending on how big a change you think this is).

    • By the way, I'm unlikely to include a breaking change unless you have a really good reason. I consider backwards compatibility really important. Consider exporting a new function and (optionally) deprecating the old one instead.

• To elaborate on motivation for the previous point: I follow a policy of releasing new code fairly quickly in most cases, as opposed to batching up a number of changes. In that situation, it makes sense for a PR to include the new version number immediately. Many other projects work differently, and do not encourage contributors to do version bumps.

  • Also, I sometimes forget to make a new release after merging a PR. If I do forget, don't be shy about pinging me to make a release.

• Include a @since annotation in each new identifier, including the new version number you just bumped to. This is absolutely vital for users of a library to properly specify lower bounds in their dependencies easily.

  • Don't include @since for un-exported identifiers.
  • If you are exporting a pre-existing identifier that was previously not exported, include the @since.
  • In other words: @since indicates when it was added to the public API, not when it came into existence.

  Example:

  -- | Download a JPEG image.
  --
  -- @since 1.5.2
  downloadJPEG :: MonadIO m
               => Request -- ^ URL to download from
               -> m JPEG
  

• Include a ChangeLog.md entry. If the project doesn't have a ChangeLog.md, create one. Odds are the content you write in the changelog can be identical to the pull request description. This is a huge end-user convenience. Example:

  ## 1.5.2
  
  * Added the `downloadJPEG` function
  

• Slight exception to the above: if you're making a doc-only PR, don't bother with a version bump, but instead add a ChangeLog entry with ## Unreleased and a description of the change.

• Do not include unrelated changes in your PR, it makes it difficult to review, more likely to get delayed, and more likely to conflict with other changes. Include separate changes in separate PRs.

• Keep coding style consistent with the rest of the code. I'm not a big stickler for coding style guidelines in general, but I do consider it very jarring to have conflicting styles in the same file, or even worse the same function.

• Similar to the previous point: think hard before sending a pull request to modify the style of code. Again, I'm not a big stickler on coding style, and I consider style in general a pretty arbitrary and unimportant aspect of the code. But that makes me even less likely to want to spend my time reviewing and discussing changes like whether records should be aligned. (For the record, I don't think they should be aligned, as it makes diffs noisier than necessary.)

• If you have a PR for addressing a typo, making a trivial fix, or adding a straightforward feature: just send a PR without any prior discussion. However, if you want to make a major overhaul, change behavior, or break API compatibility: it's worth opening an issue first to discuss. I don't like rejecting PRs or causing people to waste their time, so I'd rather hash out details before you spend a lot of time on something.

• Don't use partial functions. I've received lots of PRs which, for example, use fromJust because "by analyzing the rest of the code, you can tell it will always be a Just value and not Nothing." That doesn't cut it for me:

  • I'm lazy, and I don't want to reason about the code. I want the compiler to do it for me.
  • I'm stupid, and I don't trust my own reasoning.
  • Such logic does not withstand future refactorings of the code, making it fragile.

  There are definitely some exceptions to this rule, but odds are pretty good your case won't be one of them :).

• Added test cases are always a good thing. They also make it easier for me to understand why you're writing this PR, and ensure I don't break your work in the future (remember from the previous point: I'm stupid).

• When submitting PRs to libraries (as opposed to an application like haskellers.com):

  • Keep compatibility with older versions of dependencies whenever possible. I try to keep as broad a range of potential package versions as I can to help avoid "dependency hell." (For the record: dependency hell is not related to tooling in any way, it's an intrinsic aspect of having dependencies.)
  • In some cases, if you drop compatibility with a major version of a dependency (e.g., change from transformers >= 0.3 to transformers >= 0.4), I may consider it a breaking change in the library worthy of a major version bump.
  • Avoid adding dependencies. I personally am not of the opinion that reducing the dependency footprint is that important, and strongly believe that such behavior in general leads to Not Invented Here (NIH) syndrome. However, since enough users of my libraries do feel this way, it's easier on me if you don't incur unnecessary dependencies.