Contributing to opensource

So I have recently contributed to an opensource project for the first time. It's nothing big. It's not all that important. But it feels important to me and I want to write about it a bit.

How did we get here?

Well, it started by me deciding I could use some music making in my live from time to time. One thing that might seem like a bit of an issue at first is my obvious lack of any sort of musical instrument. We live in the 20th century however, and MIDI can get you quite far.

MIDI is the way then. I'm sure that there is a lot of (at least one) great FOSS MIDI composing tools that I could have used, but I'm lazy. I have no music experience at all, so I would need to learn a bunch of terminology and UI conventions and then be overwhelmed by a bunch of values that mean nothing to me while looking for the one variable that I'm actually qualified to touch at my skill level...

I'm a traditionally taught programmer; when I want my computer to do something, I write it to a text file and then give it to some other program from the CLI. I like it this way and I would like to be able to apply this workflow to my music making as well.

And so I searched...

And I found Alda!

Alda is a programming language (technically) that compiles into MIDI. It is written with ASCII symbols. It even has a nice NeoVim plugin that can interact with a running instance via the nrepl protocol.

Overall, While I had no Idea what was I looking for, Alda is very close.

There was however a small bug, that got me quite annoyed. I will need to explain a bit of Alda here.

This plays a bunch of notes

c d e f

This plays a bunch of notes two times:

[
   c d e f
]*2

This plays a bunch of notes two times, but some parts are played only in certain repeats:

[
   c d e f
   [ g a ]'1
   [ a b ]'2
]*2

And this is a nested repeat:

[
   [ c d e f ]*2
   [ g a ]'1
   [ a b ]'2
]*2

If you entered this code in Alda older than 2.3.2, you would have noticed that it always plays the '[ a b ]' part and never the '[ g a ]'. After a bit of experimentation, I have realised that Alda only stores a single repeat counter that is shared across repeats.

You can imagine that this can be a bit annoying, especially since I decided to not use nested repeats at all, just in case. And since I'm used to not writing the same code twice whenever possible, I decided that it bothers me a lot.

And what do you do when a bug in a piece of software bothers you a lot?

That's right! You file a bug report. In this case a GitHub issue.

But before you do that, you want to check if someone had already reported it. And in my case, someone did... 4 years ago.

So the devs obviously don't care all that much. I mean sure, it all originates from some obscure music notation that didn't allow nesting anyways, but I want my nested loops either way.

So if no one does it, I will

I mean sure... I don't know any Go at all and I have no idea how Alda even works, but the problem sounds easy enough to fix.

Alda lists the following steps in their contribution guide:

Of course, if you contribute to some repo, you should also star it. It's a good practice.

So first step is to clone the repo. Check.

Second is to actually fix stuff. Well, that takes a bit more effort.

I started by searching the repo with ripgrep for 'currentRepetition', which was my one lead from the issue.

This led me to '/client/model/repeat.go' and the following code:

// UpdateScore implements ScoreUpdate.UpdateScore by repeatedly updating the
// score with an event a specified number of times.
func (repeat Repeat) UpdateScore(score *Score) error {
	previousRepetitions := make([]int32, len(score.CurrentParts))

	for repetition := int32(1); repetition <= repeat.Times; repetition++ {
		for i, part := range score.CurrentParts {
			previousRepetitions[i] = part.currentRepetition
			part.currentRepetition = repetition
		}

		if err := score.Update(repeat.Event); err != nil {
			return err
		}

		for i, part := range score.CurrentParts {
			part.currentRepetition = previousRepetitions[i]
		}
	}

	return nil
}

// DurationMs implements ScoreUpdate.DurationMs by returning the total duration
// of the event being repeated the specified number of times.
func (repeat Repeat) DurationMs(part *Part) float64 {
	durationMs := 0.0

	for repetition := int32(1); repetition <= repeat.Times; repetition++ {
		previousRepetition := part.currentRepetition
		part.currentRepetition = repetition

		durationMs += repeat.Event.DurationMs(part)

		part.currentRepetition = previousRepetition
	}

	return durationMs
}

The issue suggested to replace 'currentRepetition' with a stack, where each repeat block will add and pop values as needed. This sounds possible, but after looking into the rest of the codebase a bit more, I decided that I'm not confident enough to not forget some part of the codebase and break a bunch of things.

Instead, I opted for a bit more straight forward approach:

// UpdateScore implements ScoreUpdate.UpdateScore by repeatedly updating the
// score with an event a specified number of times.
func (repeat Repeat) UpdateScore(score *Score) error {
	previousRepetitions := make([]int32, len(score.CurrentParts))

	for i, part := range score.CurrentParts {
		previousRepetitions[i] = part.currentRepetition
	}

	for repetition := int32(1); repetition <= repeat.Times; repetition++ {
		for _, part := range score.CurrentParts {
			part.currentRepetition = repetition
		}

		if err := score.Update(repeat.Event); err != nil {
			return err
		}
	}

	for i, part := range score.CurrentParts {
		part.currentRepetition = previousRepetitions[i]
	}

	return nil
}

// DurationMs implements ScoreUpdate.DurationMs by returning the total duration
// of the event being repeated the specified number of times.
func (repeat Repeat) DurationMs(part *Part) float64 {
	durationMs := 0.0
	previousRepetition := part.currentRepetition

	for repetition := int32(1); repetition <= repeat.Times; repetition++ {
		part.currentRepetition = repetition
		durationMs += repeat.Event.DurationMs(part)
	}

	part.currentRepetition = previousRepetition
	return durationMs
}

Yes, it allocates new memory each time a new repeat is called, but that isn't all that often. Also yes, Go is garbage collected, so there is no memory leak.

So fix Check?

Now to the tests. I have looked into a few tests and all tested converting syntax to instructions and not instructions to MIDI, so it didn't seem like writing a test for this was possible.

Now submit a pull request. Check.

Ok, now we wait...

So basically Dave Yarwood responded that he'll get to it soon and if I can make some tests in the meantime. So I looked into the tests again and guess what? There were tests for instructions to MIDI conversion.

And so I copied some syntax and made a simple test.

And it got merged. So nested repeats work now. good.

...

...

So I guess I contributed to opensource. Well, this was always advertised as one of the main benefits of opensource, but it always felt to me like the thing more experienced devs do and what I don't have much rights to attempt.

But I guess it's not that hard. Well, you don't even need to know the language it seems.

IDK, it mostly just feels cool and I wanted to share my thoughts, which are mostly that it feels cool, so I don't have much to share.

Peace!