Getting My First PR Merged

I recently had my first non-trivial PR merged into an open source project, and I was hoping that my story could help you feel empowered to contribute as well.

Open Source will find you

First, I want you to feel empowered, not indebted, to get involved. Despite what we may feel (or be told), you can be a perfectly good developer without ever making an open source contribution.

I was helping my students debug some DB code, and I noticed a specific error message that I thought could be improved.

(sqlalchemy.exc.InvalidRequestError) A value is required for bind parameter 'id' [SQL: 'select\\n    *\\nfrom\\n     reviews\\nwhere\\n    id = %(id)s'] (Background on this error at: http://sqlalche.me/e/cd3x)

To be clear, I think this is a good error message. It even includes a shortened link for finding more help online. I just thought it could be improved.

Scratch an itch if you have one. If you feel like you don't have anything to contribute to a project. That. Is. OK. You are already doing good work.

Baby steps

I had two primary pain points:

1. Terminal wrapping is no fun. Maybe a little work could be done to split the error message up more gracefully over multiple lines.
2. The relevant SQL statement looks a little different than it was written. I thought some work could be done to make the SQL more easily recognizable.

So the above example would have turned into something like the following:

(sqlalchemy.exc.InvalidRequestError) A value is required for bind parameter 'id'
[SQL: (select
    *
from
    reviews
where
    id = %(id)s)]
(Background on this error at: http://sqlalche.me/e/cd3x)

Awesome, I have a clearly-scoped itch to scratch. Finding the code responsible didn't take too much time, and it was so nice and small. I had to change two tiny lines.

        details = [self._message(as_unicode=as_unicode)]
        if self.statement:
-           details.append("[SQL: %r]" % self.statement)
+           details.append("[SQL: %s]" % self.statement)
            if self.params:
                params_repr = util._repr_params(self.params, 10)
                details.append("[parameters: %r]" % params_repr)
        code_str = self._code_str()
        if code_str:
            details.append(code_str)
-       return " ".join(["(%s)" % det for det in self.detail] + details)
+       return "\n".join(["(%s)" % det for det in self.detail] + details)

I had to change a %r to a %s and a " " to a "\n".

It might take a little work to find the relevant code, but a novice programmer can make this change.

Nothing is that simple

The project in question, SqlAlchemy (https://github.com/sqlalchemy/sqlalchemy/), is a popular Python project that supports Python 2 and 3. So I hit a snag related to cross-version support that I wasn't really prepared to solve on my own. Also, a good handful of tests broke for such a small change. Take a peek at the final PR:

Issue #4500 : Change StatementError formatting (newlines and %s) #4501

natec425 posted on Feb 17, 2019

This changes the error formatting for StatementError in two ways:

1. Break each error detail up over multiple newlines instead of spaced out on a single line. Hopefully, this helps readers scan the error message more easily.

2. Change the SQL representation in the message to use str (%s) instead of the current behavior that uses repr (%r). This should help readers recognize the structure of their SQL, particularly if it is a multiline SQL statement. In the multiline case, the SQL would be printed over multiple lines instead of printing an escaped "\n".

Fixes: #4500

This pull request is:

• [X ] A short code fix
  • please include the issue number, and create an issue if none exists, which must include a complete example of the issue. one line code fixes without an issue and demonstration will not be accepted.
  • Please include: Fixes: #<issue number> in the commit message
  • please include tests. one line code fixes without tests will not be accepted.

View on GitHub

That small diff ballooned a bit.

You will fail a CI build. It is OK.

But there is help

Thankfully the project's maintainer was super helpful and friendly every step of the way. I didn't feel comfortable digging into the python2-3 issues, so he stepped in and sorted that piece out. If it was up to me, I would be left with "So when is SqlAlchemy dropping support for python2 😄?". So that brings me to my last takeaway.

Solving the problem is not the goal.
Being a part of the team is the goal.

Repeating myself

1. You can contribute. Don't feel like you need to. Your own work is important.
2. If you notice an opportunity, remember that you can do it.
3. It will probably be difficult in ways that you didn't expect.
4. You are not alone. Hitting a road bump is part of being on the team.