It took three weeks, but I finally landed my first pull request. Here’s roughly how that works:

Step 2: Ok, Get Started

I have my fork, I have my local branch. What should we do first?

Screw it up!

I figured that I wasn’t going in completely blind, since I had used error-chain once. I may well have asked how hard, say, healthcare could possibly be. Just to pick an example. At random.

Any kind of major refactoring consists of one part thinking through a very small change, and a hundred parts typing, and typing, and typing until the compiler is happy. Then, if you’ve been very good all year, Santa Clause will bring you an automated test suite and you get to make that happy too.

The act of refactoring is less interesting, so I’ll hit some highlights.

Macro-ergonomics

In any language with a macro capability, you will end up writing a thing that looks nothing like the code it will actually represent in the end. The challenge is understanding what the “real” code underneath looks like so you can write about it elsewhere.

For example, to declare a new error type, I would write something like:

error_chain! {
+++ errors {
++++++ MyError(bargle: String) {
+++++++++ description(&bargle)
+++++++++ display("{}", &bargle)
++++++ }
+++ }
}

Since error_chain is a macro, the syntax for this doesn’t resemble real Rust syntax, so the only documentation on how to really use it are examples from which you must infer correct usage. If you’ve got it wrong (and I spent the first two nights getting it wrong), the only thing the compiler will tell you is that the code that the macro expanded into has a syntax error. Worse, it’s an error that occurs at a phase where it blocks things like the pretty printer that could show you the code expansion.

Speaking of…

So Pretty

Cargo has an available sub-command called expand that invokes the underlying compiler pretty-printer functionality. What this does is run the macro expansion portion of the compiler and dump the translated code to the console. Assuming that you’ve made your macro syntax sufficiently happy, you can now inspect the code generated by your macro invocation. I relied on this pretty heavily in order to know what I actually could write.

Just Break Git While You’re At It

I was rolling along pretty well with my changes, until a few days later when I looked around and realized that the world (or at least the upstream) has been moving along without me. Ok, I can figure out how to do this.

git fetch upstream

git merge upstream/master

Hmm… I’ve got lots of checkins, and now here are other people’s checkins between the front half of my work and everything else I’m going to be doing. Won’t that be messy? Hey, there’s this rebase thing.

I chose… poorly.

Something got mangled, and the next night I decided to do the drastic thing: blow away my current local repo, re-fetch latest from upstream, and manually “merge” (copy) my local edits back into the new repo.

Step 3: Coming In For a Landing

Finally, I have the compiler happy. There are some tests that are broken, but I’ve been careful about my changes so that they are supposed to preserve the existing program semantics. This means that I can be pretty sure that test failures means I need to fix code, not change tests until they think my code is right.

In the meantime, the project’s lead says: Open a pull request, and I can start reviewing while you fix up tests.

Here we go: git push will send my local copy back up to my github fork, and from there it’s a few clicks to open a Pull Request on the original, asking them to accept my code changes.

That’s two weeks of work down, and another week to go fixing things up…

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s