class: title name: title # Through the Eyes of a Maintainer ## How to Make Your Contribution More Successful ??? --- ## Failed contributions  ??? As a contributor, you don't want to see your work immediately rejected --- ## Failed contributions  ??? Or sitting in limbo for years --- ## Failed contributions  ??? Or with so much back and forth that you give up Whether new to programming or new to open source, the role of an open source maintainer is unfamiliar. My hope is that by showing things from the perspective of being a maintainer, you’ll be able to better understand how to work with them and not fall into those PR traps I showed earlier. However, not every suggestion will be 100% applicable to every project you interact with. Each maintainer is different, with different values and --- class: title # Humility ??? Humility is an important trait when contributing I’m not talking false humility, to downplay yourself, to pretend to be less than you are, or to not be willing to speak up for your needs. I’m talking about recognizing there might be other approaches, other priorities, other value systems. For example, when I disagree with something someone posted in an Issue or their PR, I ask myself “can two competent engineers disagree on this?” If so, how much does it matter? --- class: title # Communication ??? An important skill is clear communication. I know the Rust project is English-centric and I am not in the United States. I am speaking less about spoken and written English skills but how you communicate. --- ## My TODO List - Cargo: MSRV-aware resolver - Cargo: cargo-script - Clap: dynamic completions - Toml: faster parser - Snapbox: improve directory snapshotting - Changelog fragments ??? I try to focus on the large, hard to define problems that are harder for others to pick up and run with. However, that also means I need a lot of focus time to make progress. I also write software because I enjoy solving these problems. --- ## Everyone else's TODO list for me  ??? I need to balance that out with - supporting users - triaging bugs and feature requests - fixing critical bugs - mentoring contributors on designs and PRs Also, the cost of context switching adds up. For any given project or domain within a project, it takes time to refresh myself on design and the concerns with it. I compensate for that using focus areas. I select a project to focus on and prioritize my work on it and other people’s issues and PRs. All other projects only get triaged in case there is a critical bug or I can easily unblock someone. And this is my job. Others need to balance this with their day job and family. --- ## Engage helpfully  ??? This doesn’t mean “don’t bother maintainers” but be productive and positive. “Me too” posts are at best unnecessary noise and a distraction, taking time away from a maintainer who is already time limited. Maintainers aren’t your employee. Telling them what they must do also isn’t helpful. What is productive? Sharing new use cases that help shed light on why a feature is important or on new requirements, summarizing a long discussion so its easier for anyone, including the maintainer to catch up, etc. --- ## First PR!  ??? Let’s now try to contribute. You have an idea and post a PR. See how large that is? What will the maintainer think? - You cared about something, showed up, and invested a lot of time into it - But maybe its not the right direction? The maintainer now is put in the awkward spot of having to throw out your work. - Or what if it wasn’t a focus area of the maintainer? They now either need to shift priorities and context switch or ignore everything you put into it. PRs put social pressure on maintainers to move them forward. - This can lead to stress that can burn out a maintainer - It can also bypass the queue of people who were trying to get buy-in from the maintainer first. --- ## Why closed?  ??? For anything but trivial PRs,maintainers need to understand the the use case and requirements. From there, we explore solutions that align with the project’s values and priorities and align well with other features, existing and requested Someone might walk away from the PR. We now don’t have this in our backlog. We likely won’t even find it because people tend to search for this information in Issues. --- ## Risk of PR first   ??? Someone else might go and pick up the PR but now we are picking up that conversation in a different location, splitting the conversation. My general rule of thumb is that if there is a risk that it needs discussion, it should start as an Issue. In my experience, people underestimate the likelihood that use cases, requirements, or solutions will need to be explored. Fixing of typos is usually safe. When creating an issue, focus on the use case and requirements. Its ok to suggest a solution but remember the Humility part. It might not be the right route or the right timing. --- ## Communicate!  ??? So when can you start implementing? That depends. For a larger project like Cargo, we track the next steps for an Issue with labels. --- ## First PR!  ??? Now that we are ready to write a PR, let’s go back to our previous example. Look how much changed in just one commit. This includes - The actual fix - New tests and updates to existing tests - Refactors to align the code with the new needs - Other bugs fixed along the way --- ## First PR!  ??? In a sea of green and red, you can’t easily tell the intent of the change. Instead you have to reverse engineer it starting at random points. Part of clear communication is making each change as small as possible (but no smaller) to make the intent clear. Tests should still pass. Documentation should be tied to relevant commit. We call these “atomic commits”. For one fix, I had 30-40 refactor commits and the actual big fix commit was just a single line of code. --- ## First PR!  ??? You could have easily missed that I changed the source code shown on the slide. In particular for tests, it is hard to tell with new tests what the old and new behavior are or if the test is even testing the right behavior. I know I’ve written tests where I found my bug fix didn’t make a difference on whether --- ## Split commits   ??? For tests, split out a test commit before the fix that shows the existing behavior (ie it passes). Then when you make your fix, you only change the test for the part that changed. This makes sure you are testing the right thing. Reviewers and the wider community can easily see what the intended effect of your change is. Much more clearly than a long description in a PR. --- ## First PR!  ??? So now we went from 1 commit to 22 but that isn’t the end of the story. Now, what happens when a PR is this big? - Its at risk to merge conflicts - Other work may be blocked on parts of it - The larger it is, the more review comments, the harder it so track the comments, making it take more work for each round of reviews from maintainers, and more rounds of back and forth - The larger it is, the easier it is to miss things which will slowly be found after each round of reviews, causing more rounds of reviews - The more changes done in isolation, the more likely later commits will need to be redone or discarded. --- ## Split PRs    ??? So for the example PR I gave, it turned out it was split across 7 PRs with 101 commits They ranged from 2-34 commits in each PR So how much do you fit into a PR? 1 unit of of controversy and any commits that don’t make sense without that commit. Some refactors are always useful but some have a neutral affect or make the code worse until another change comes along and uses it. Sometimes I split mid review as I see where things are going. My 34 commit PR? No review comments. Not because of how large it was but because of it was uncontroversial. Judging controversy takes experience with the project and your reviewers. --- ## Communicate!  ??? When it comes to review comments, be sure to speak up! They are a conversation, not a series of instructions - You are likely the most recent person to have touched this code. The reviewer might be a bit rusty - You are likely the only one with direct experience with your change If there is a concern over feedback, say so! If you start implementing an idea and its not working out, say so! Don’t force the feedback to work Either the reviewer missed something or there was a misunderstanding with their feedback. Assuming all goes well, your PR is merged. For the Cargo team, we try to remember to recognize people’s contributions in our per-release blog post.