Rust Audio

Management & Processes

We will need a written plan of action to follow certain requests. We currently (and unspokenly) get 1-3 reviewers (depending on the size of a patch) to merge something into master.

For admins on GitHub, I’d propose the following review approval numbers, based on the scope of the PR. Note that the person taking this action counts as 1 approving review.

For example, if you were to create a PR to fix a typographic error, you can merge it immediately, since you are implicitly the 1 approving review needed to merge.

Type of PR Min Approvals
Typographic correction or phrasing in documentation or associated text 1
Non-breaking small code refactor or feature implementation 2
Any Medium to Large size change 3 - 4
Any breaking change 4
Any large feature 4

The catch here: These numbers are arbitrary for the most part. I’m not a trained software dev or project manager, so if you have a better solution please bring that up! The above is more or less an example.

This sounds good, with the addendum that I think API-breaking changes (and maybe “large features”) should probably be approved by at least one person from each of the large OSes (Windows, Mac OSX, Linux) just to double-check that it didn’t break anything. I don’t think anything would break at this stage, but you never know.

It would be nice to be able to check with a bunch of different DAWs, but maybe we don’t have the resources quite yet for this…

I think we could probably use the “label” feature of GitHub to specify what number/type of reviews are required before merge, just so someone can immediately look at the PR and see where they can help out.

Also, as mentioned in #81, I think it would additionally be helpful to have some sort of release plan so we can start to prioritize work and releases better. We could then use the “label” feature for issues as well so people know what work that isn’t done is highest-priority, and they can help work on those things as well.

If our current trend of how hard it is to get people to review things continues, we will never get anywhere with a process as heavyweight as this.

We are a non-safety-critical crate at version 0.whatever. We should be far less afraid of breaking things than what is the state currently. If we do a change and it breaks something, we will just fix it and move on.

I think one of the reasons people are afraid of breaking things is that our test coverage is pretty bad. We should work on improving that and then just hack away on the code.

2 Likes

Agree with all of this.

I will mention that we’re considering splitting the crate into vst and vst-sys, maybe that would also be a good time to think about our testing too? (we need to have a process for that too…)

1 Like

Yes, @askeksa makes a good point here. We need a better way to review/merge as well as more comprehensive tests. To get there, we should have a roadmap as it’ll let us agree on a lot beforehand, and then just leave it up implementation.