104

While going through the README file of a GitHub repository I am not a contributor of, I noticed a few minor typos and wondered if I should submit a pull request to correct them or if reviewing the request would take the maintainer too much time to be worth it. I considered the three courses of action:

  1. Correct the typos and submit a pull request with the position of each correction in the summary field.
  2. Send the maintainer an email with the proposed corrections.
  3. Do nothing.

Which of these (or any other) options is most appropriate?

To give some context, the repository is actively maintained and has about 10 contributors. The typos I have noticed do not make the README misleading nor ambiguous. I have used GitHub for some time for small personal projects, but have little experience with pull requests and how much time and efforts they take to review (hence my question).

Improve this question
10
  • 56
    Whatever you do, don't mention t-shirts
    – Basilevs
    Nov 17, 2020 at 3:50
  • 3
    PR is great. I would make sure to only submit clear and undoubtly wrong corrections in a single P? If you also want to optimize the wording or fix formatting, do it in a separate run. This ensures at leas the typo changes are accepted quickly. Check with the contribution guidelines first, some projects might require CLA-like procedures even for minor fixes (which IMHO is a bummer). If a maintainer prefers to fabricate their own commit, they can start from the PR, so this is a good workflow as long as the project is actually working with GitHub tools.
    – eckes
    Nov 17, 2020 at 4:13
  • 4
    @FlorentMichel No, an emailed patch file is not appropriate for contributing to a project maintained on Github either. It's not quite as bad as sending a list of description, since it uses git versioning and means that the maintainer doesn't have to do any file editing themselves, but it still doesn't use Github as the collaboration platform where pull requests are very simple to apply.
    – Bergi
    Nov 17, 2020 at 9:32
  • 8
    @Basilevs Why would OP mention t-shirts?
    – gerrit
    Nov 18, 2020 at 8:19
  • 12
    @gerrit It's a reference to the Hacktoberfest 2020 debacle: infoq.com/news/2020/10/hacked-off-hacktoberfest
    – marcellothearcane
    Nov 18, 2020 at 15:44

5 Answers 5

Reset to default
182

Just fix all the typos you noticed and create a pull request with a comment along the lines of 'Fix typos'. Then it's one button to click for a person with the correct access.

You don't need to explain each and every typo; it will be clear in the diff itself.

Getting information about the typos from the e-mail will be harder to apply for the developers (and may be even harder for you).

Improve this answer
9
  • 23
    Yup, just correct and submit a PR. That's the whole point of distributed development. They have the option of approving and merging, or not. As someone who has approved PRs similar to this, I appreciate that someone took the time to make these corrections, because I didn't have the time to do it, or review the readme good enough.
    – Greg Burghardt
    Nov 16, 2020 at 14:30
  • 17
    The easier it is for those that are not you, the happier they are.
    – Thorbjørn Ravn Andersen
    Nov 16, 2020 at 15:06
  • 4
    “it's one button to click”: That's exactly the point I was not sure about. Thank you for clarifying it!
    – Florent Michel
    Nov 16, 2020 at 20:27
  • 120
    Just make sure you really are "fixing typos" and not doing something like "converting British English spellings to US English" (or vice versa!) which might not be appreciated!
    – alephzero
    Nov 16, 2020 at 21:02
  • 3
    There is one situation where I would (and have) chosen the option of opening an issue describing the necessary changes instead of making a pull request: when there are complicated contributor guidelines that include setting up a complex environment. This is less of an issue for a readme, but it could still make me do that.
    – Jasper
    Nov 16, 2020 at 23:32
55

Some context that may or may not be relevant.

A cloud hosting provider named DigitalOcean hosts an event every year called Hacktoberfest to encourage people to contribute to open source projects in exchange for a T-shirt. Wonderful as it is to encourage more people to get into open source, offering a reward in return for gameable metrics and packing it all into one month resulted in many projects receiving a flood of low-quality pull requests that were perceived by some maintainers as spam. A number of these involved trivial changes or vandalism to README files, some inspired by a demonstration on a YouTube channel.

At worst, we're talking about things like this "improve docs" PR, which only added unnecessary periods to comments, or this one, which added an odd and unnecessary header to a README. It was particularly noticeable this year, with some projects receiving a dozen of these in the first day of the event, which caused a lot of consternation among project maintainers and blog posts like DigitalOcean's Hacktoberfest is Hurting Open Source.

Hacktoberfest is over, and that specific problem has abated (the amount of spam caused them to change the way the program worked after a few days to make it opt-in for project maintainers), at least until next year, but that incident gives a bit of context into how many maintainers think about pull requests: PRs exist to improve the software, and PRs that seem to exist just for the sake of changing something without a clear purpose or benefit may be frowned upon, especially in bulk. If you were only fixing a typo in a comment, seemed to be changing documentation for no real reason, or arbitrarily rewriting a bit of code without making any particular improvement, that's the sort of thing that could raise eyebrows.

But in your case, your PR seems like it would genuinely improve the software. Having multiple typos in a README, assuming they are real typos and not regional spelling differences, is less than ideal, and you're providing a material improvement by fixing them.

The fact that you're asking this question and are concerned with respecting the maintainer's time makes it clear that your intentions are noble. Ultimately, to assuage your concerns, reviewing and merging a simple pull request is a quite quick and painless operation for a maintainer—look at the diff and click a button.

Lastly, GitHub is fundamentally just humans talking to each other. It's completely ok to write something like "This is my first pull request. I hope this is useful, but no worries if it's not" or something to that effect if you're unsure.

Improve this answer
3
  • 3
    The context you provide seems very relevent! While this event is not taking place at the moment, I imagine it may have contributed to how the community, and in particular maintainers of popular repositories, see trivial-ish PRs; and it will certainly be useful to readers who may read this answer during a similar event.
    – Florent Michel
    Nov 17, 2020 at 8:23
  • "perceived by some maintainers as spam" is a bit generous: youtube.com/watch?v=MfvFRAJS9nw
    – GammaGames
    Nov 17, 2020 at 20:59
  • 1
    At first, I did not get the "t-shirt comment" below the question. Your answer helped, thanks!
    – Heinzi
    Nov 18, 2020 at 8:09
15
  1. Correct the typos and submit a pull request with the position of each correction in the summary field.

That information is already visible when looking at the changes themselves, and any pull request reviewer is obviously going to be looking at the changes that were made.

The summary field should be kept for a human-readable short summation of what this PR contains. In your particular case, "fixed some typos" accurately sums it up.

  1. Send the maintainer an email with the proposed corrections.

The entire point of GitHub is that you can cooperatively coordinate changes to source code. If emailing someone and having them make the changes were the way to go, GitHub (and versioning systems in general) would lose their primary purpose.

Whether you corrected a typo or changed some code is irrelevant as to how GitHub works and how its members coordinate changes they've made.

The only reason why I'd consider email the maintainer was to clarify something, e.g. if you're unsure if something is unintended behavior or not.

In this case, a simple PR for typo fixes doesn't really need clarification, the maintainer can look at the PR itself and decide whether they need to or not. You'd spend more time trying to get them to look at everything and asking if your correction is correct, than you would opening the PR itself.

Just doing the PR directly saves both you and them some time and effort.

  1. Do nothing.

I mean, it's technically an option, but it doesn't quite address the question on how you should approach this. The inherent implication of such a question is that you're trying to do the thing you're asking about. Doing nothing is not doing something.

Improve this answer
2
  • 19
    Including "Do nothing" asks an implicit question: "Is this issue actually important enough to do anything about?". Suppose you noticed at a public park that a bit of sand from the playground had gotten onto the grass. This is undesirable: should you write a letter to your city council recommending they fix it, or is the issue too small to be worth your and their time? Of course in that case it's not worthwhile. Fixing typos in a README is trivial, but the cost of accepting a PR is also small, so it's fine to do. But if you don't already know that, there's nothing wrong with asking.
    – amalloy
    Nov 16, 2020 at 19:52
  • 12
    Another reason why you'd e-mail the maintainer would be if you were reporting a serious security issue that shouldn't be publicly disclosed until after it's fixed.
    – jwodder
    Nov 16, 2020 at 20:28
7

Not that this specifically answers the question, as there are already answers, but here is the general advise I give people on commits and messages.

Break up commits into logical chunks. The example I use for this is if a file you need to edit has a bunch of formatting issues. You should make one commit to correct the formatting, then a second commit to fix the logic that you originally went to the file for. This way, if for some reason the logic needs to be reverted, the white space changes can stay in for the next person that needs to go in there. Ideally the formatting you fix meets whatever the local coding standard is.

As far as an individual commit, answer the questions that they were taught when writing grade school papers. Who? What? When? Where? Why? How?

Who is generally answered by the author of the commit.

When is the timestamp of the commit

What and where are generally the diff of the commit.

Why and how really need to be in the commit message. To me, this is the most important part to get right and hard for a lot of software engineers to do. In this case specifically, and since the changes are minor, how is irrelevant and why is just that you fixed typos.

TLDR; Don't worry about a commit to fix typos or formatting. Make a commit and put it out for review.

Improve this answer
5
  • "Why" should be clear from the code too. If it isn't clear from the actual instructions, there need to be comments. If the reason for the change is only apparent from the commit message, that's bad. The commit message is great for looking up prior changes though.
    – Asteroids With Wings
    Nov 17, 2020 at 16:39
  • 2
    What you did is apparent from the code, but not Why. Why did you feel that change was necessary. I've spent a lot of time looking at old code trying to figure out what they were thinking. If only they were still around to explain why that did what they did, then maybe it would make sense.
    – Ben
    Nov 17, 2020 at 17:06
  • Or if they'd explained it with some simple code comments, as I suggested.
    – Asteroids With Wings
    Nov 17, 2020 at 17:36
  • softwareengineering.stackexchange.com/questions/411585/…
    – Asteroids With Wings
    Nov 17, 2020 at 17:37
  • I agree that good comments in the code are helpful, and can even explain why something is the way it is for a small grouped code change. But when a code change is spread across several files, the developer isn't going to comment every change. Too many comments in the code is just as bad as none.
    – Ben
    Nov 17, 2020 at 21:44
2

The other answers are good, but first check if there are any instructions for contributing - in particular regarding signing a CLA. (I see that @eckes also stated this as a comment on the original question.)

If the CLA process seems too complicated it might be better to open an issue (or send an e-mail) only stating the spelling mistakes than to construct a PR.

Improve this answer
2
  • 3
    If the project uses GitHub Issues and you don't want to/can't sign a CLA to submit a PR, it likely makes more sense to file an issue rather than send an email, so it can be tracked along with all other issues.
    – Zach Lipton
    Nov 17, 2020 at 18:56
  • @ZachLipton - True - I updated the answer accordingly.
    – Hans Olsson
    Nov 18, 2020 at 7:55

Not the answer you're looking for? Browse other questions tagged or ask your own question.