In my 2020 vision, I mentioned that this year I wanted to contribute to an open source project and I am happy to announce that my first ever pull request was merged. This post mainly concerns my experience of the contribution process and my thoughts throughout rather than an in depth technical report.
When I embarked on this journey I had a few criteria that I was looking for in my contribution. Specifically, I wanted to contribute something meaningful to both me and others which meant that I wanted:
- to contribute to a project I use or found interesting
- to add value to others’ experience
- to add value to the project
With those parameters I went through the Github issues of projects I liked and checked the “good first issue” labels until I stumbled upon this one in the repository for Singularity (a containerisation tool for high performance computing on Linux). The issue stood out to me, because I had also stumbled into the same headache as the author and it seemed to me to be a very achievable thing to fix.
At first, I was tempted to comment on the issue that I could look into it, but a teacher of mine once told me, “talk is cheap”, and so I decided to instead produce a solution first.
Doing My Homework
Although I had a vague idea of what the process was for contributing to open source projects, I also know that each project might have a subtly different workflow for contributions. As a result, I made sure to read the contribution guidelines for the project, and forked the repo.
The first thing to do was to ensure that I was able to actually compile the project from source, which meant understanding the build system and dependencies. Luckily, working with lots of build systems (and even writing my own makefiles) meant that it went smoothly and soon I had a workflow figured out.
Finding the Problem
At this point, I had reached the limit of my comfort zone. The project is written in Go which I had very little experience with having only completed the Tour of Go previously. Furthermore, I was not at all familiar with the codebase and so my first issue was finding where the problem could actually be solved.
Thankfully, I am a bit lazy and instead of reading through each and every file
to find something I ended up using a recursive grep search to find the
specific occurrence of the sanity check prompt.
$ grep -r "Build target already exists"
cmd/internal/cli/build.go: question := "Build target already exists. Do you want to overwrite? [N/y] "
So I dove into the build.go file and found the line where the prompt was
being served, but if I was really clever I would have added the -n option to
grep which would spit out the line number as well as the file it was in.
$ grep -rn "Build target already exists"
cmd/internal/cli/build.go:268: question := "Build target already exists. Do you want to overwrite? [N/y] "
Initial Solve
My first thought about solving this was to check if the build target file had
the .def extension that denotes it as a build file. However, there were two
big issues with this approach: it depended on the name of the file rather than
the content, it required introducing the os package to the file which I
thought was unnecessary.
Looking through the file for tools I could use I came across the following
method parser.IsValidDefinition(path string) which would judge a build target
file on its content rather than its name. Furthermore, it incorporated the
internally built tools to check definition files. This lead me to the following
diff:
1diff --git a/cmd/internal/cli/build.go b/cmd/internal/cli/build.go
2index fe02643162..558a7a2bfc 100644
3--- a/cmd/internal/cli/build.go
4+++ b/cmd/internal/cli/build.go
5@@ -265,7 +265,17 @@ func checkBuildTarget(path string) error {
6 return fmt.Errorf("only sandbox update is supported: %s is not a directory", path)
7 }
8 if !buildArgs.update && !forceOverwrite {
9- question := "Build target already exists. Do you want to overwrite? [N/y] "
10+
11+ var question string
12+
13+ isDefFile, _ := parser.IsValidDefinition(path)
14+ if isDefFile {
15+ question = "Build target is a valid definition file that will be overwritten. Do you still want to overwrite? [N/y]"
16+ } else {
17+
18+ question = "Build target already exists. Do you want to overwrite? [N/y] "
19+ }
20+
21 input, err := interactive.AskYNQuestion("n", question)
22 if err != nil {
23 return fmt.Errorf("while reading the input: %s", err)
What followed was a bit strange for me. I pushed the new commit/branch to my fork with the commit message “Add build command sanity check to close #3502” with “#3502” being the reference to the issue in the main repo. I did not expect this to immediately reflect on the issue in the main upstream repository. So when I amended my commit because I forgot to ensure all the test cases, and builds passed, it also reflected multiple commits on the main issue conversation. Although unexpected, this is very useful for connecting commits to issues automagically.
Excited and thrilled, I opened a pull request and waited for a review.
Feedback Loop
The pull review process was also a novel experience for me. Initially I was worried that my changes would be outright rejected, and my PR tanked. Instead, there was a back and forth discussion between myself and David Trudgian — a maintainer for the project.
The conversation about how to address the problem resulted — at least in my view — in a solution that was greater than something I could have achieved by myself. I would incorporate suggested changes and neaten up my code which ended up in a total of three commits for the pull request. The review process felt very welcoming and collaborative.
Victory
With all suggestions incorporated, continuous integration and build tests passed, my PR finally passed review and was merged into the master branch.
While I believe that the contribution I made was definitely beneficial to future users of the software, I definitely walked away from this experience with a lot more insight. I experienced the power of the review process, and that contributing to open source is not only achievable, but exciting and enjoyable.
I definitely plan on making more contributions to Singularity in the future, although I suspect not all of them will warrant a full blog post.