Introduction
Over the past years, I contributed to various projects on GitHub. It turned out two recent contributions (1 & 2) turned into fiascos. Fiascos I’ll try to comment here, as they retrospectively appeared to me as “weak signals” of FOSS ecosystem deep symptoms related to contributions acceptance.
In this post I’ll be referring a lot to “FOSS”, standing for Free and Open-Source Software.
Responsibility vs. Accountability
It’s true maintainers are accountable for the software they work on. Although, as an external contributor I am definitely responsible for the (broken ?) patch I push.
If you fear accountability, despite the “zero guarantee” OSS license that should already protects you (strictly speaking from a legal point of view), feel free to reject patch soon (or you can always rework it yourself, if you prefer when “it’s written your way”).
Don’t be picky about testing if your project isn’t (yet ?)
Don’t put the burden of missing integration/non-regression tests on contributors ; They already invested a long time to :
-
checkout your code and setup development environment ;
-
read your documentation (if any) ;
-
nail down the bug/implement a new feature ;
-
read your contributing guidelines (if any) ;
-
follow your coding style ;
-
cleanup the patch before proposing it to the world ;
-
deal with your forge technical specificities to actually submit changes.
Optionally, they could also figure out the issue required to be addressed in a third-party dependency, or even worst case : a fork you made of it (!).
Focusing on improving “tests” could be handled afterwards, as a totally separate concern (proposing a bug fix or new feature on a currently untested code base is fully independent from addressing the actual issue).
Let’s be clear : improving an untested piece of code is still improving a piece of code. End-users (including third developers) absolutely do not care about the status of your current code base coverage.
If your entry point (main
) isn’t tested (only integration tests can), please exclude it from code coverage results, allowing it to be modified (read “improved”) without bringing “bad statistics” to your CI.
FOSS workload
This is not new : there is only a few people maintaining way too many software.
One of the consequences of these bus factors (also called “circus factors”, like the recent decision of Neofetch creator and maintainer to simply quit after more than two years of vacancy) : it’s hard to expect from them fully-rational and situational reviews (i.e. reviews based on the actual diff instead of whether or not the CI pipeline is green, commits have been “signed-off” or overall code coverage has increased).
Code bases are huge and usually rather complex (we wouldn’t propose changes if they weren’t), thus they often require maintainer complete focus to be properly treated.
(opinionated) FOSS philosophy
As a maintainer, you’re committed to the project roadmap. But if you adhere to Open-Source Software philosophy that’s only one of the missions.
Let’s name a few others :
-
make it work in most of (supported) environments, mostly when a contributor already did the job and implemented another platform or environment support ;
-
make it as secure as possible because, despite the “zero guarantee” license, you don’t know what your software will be used for, and it may end up in a critical system (that’s why EU funds OSS bug bounty or USA now considers OSS security as critical) ;
-
provide help and support for users (at least by pointing them to documentation or third resources so they can properly work on their side) ;
-
explore any possibility of “empowerment” : if there is a need, or when developer opted for an opinionated default behavior, there should be an option for it.
Hot take : if you don’t think so, but still are enthusiast about publishing some lines of code (because you like to share stuff to the world, or think code is a form of art), please push them to a read-only mirror or disable direct contributions on your forge (this will spare some developer precious time and make them opt for an alternative solution to address their issue).
About “repetition bias”
It’s not because there are plenty of idiots sending you harsh or dumb messages that a new contributor will be necessarily harsh or dumb. You don’t have to repeat four times the same thing : we usually also have a brain behind our screen.
If you will eventually refuse proposed changes, don’t ask three times for a new version (believe it or not, it actually consumes time on the other end) and quickly (not after 6 months) reject them by explaining why.
About upstream testing
Do not ask me to test third-API, mostly standard library ones. They are usually already tested upstream, and if not (shame on their maintainers !), they are de facto (due to the number of programs using them for several years and the fact they still exist in their current form).
Instead, you should add linting jobs to your CI, which will check for their proper uses (number of arguments, possibly their types, return type, …).
Proofs of “working patch” must not come from contributors themselves
Please do not ask me to “record videos” of my screen showing that submitted patch “work as expected”. This has absolutely no value, and it consumes time.
You can’t possibly have any trust in my machine, the code I am (showing you) running, the configuration I dumped somewhere on disk nor the runtime environment it uses. It isn’t reproducible at all, and gives a false feeling of safety.
You want to make sure proposed changes “work” and you don’t have any non-regression test ? Please kindly checkout my branch and run your own tests locally. You’re likely setup to do so, and if you think your setup is representative enough, you’re free to accept changes and merge them.
In cryptography, we don’t trust a peer handing over a public key without a signature from its private part.
More generally in science, we don’t consider something true until it has been demonstrated.
Be concise, avoid low-value comments
… including moral judgment and/or proselytism.
Serializing information (from brain to a text box) and deserializing (from an HTML page to brain) costs us time and energy. Please don’t use issue tracker or review comments to share your rants like :
[…] You need to [understand] test, “I think” is the worst [word] for testing.
… which had been quickly edited afterwards to :
[…] You need to [understand] test, testing is for fixing your “I think”.
Yes, thanks, I understand tests quite well. I implemented unit and integration tests from scratch for an existing code base, and always include proper jobs in new projects. All my contributions address documentation and tests, when it’s possible (read “when your project already got some”).
(While we’re at it : my “I think” wasn’t about testing itself, but precisely the fact that “testing videos” recorded on third-machines cannot act as tests themselves.)
“You’re right, but since the xz backdoor incident we must be careful”
Contrary to way too many members of the FOSS community, I don’t consider the “xz backdoor attempt” as a failed one. Even if it didn’t reach Debian stable
repositories (or any other Linux distribution “stable” release) nor each and every running OpenSSH servers on the planet, it actually end up running on systems connected to Internet, hence machines that we can consider “compromised”.
One may always say that running production systems on “unstable” or “rolling-release” distributions is a terrible idea (and I would agree here), but this changes absolutely nothing : malicious code has been executed somewhere without CPU owner will, and that is definitely a “success” from perpetrator(s) point of view.
Moreover, if the supply-chain attack actually targeted an organization running even one public-facing SSH server using a “rolling-release” distribution, it could be considered compromised.
If we take Debian for instance, it has been almost one month between the first merge of malicious content (it appeared “broken”, from perpetrator(s) point of view) and the revert to a previous (supposedly) “clean” version.
One, month. Think about it.
Indeed, we must be careful about what we merge. That’s why integration pipelines must be as relevant as possible and security tests should also be included. But automation won’t solve any problem (as always), and brain needs to be switched on to give a contextualized review.
Conclusion
Maintaining a software is accepting that we (maintainers) are not the only ones to use it, nor to decide what it should look like. It also means others’ needs have to be treated like our (their) ones. The direct consequence of this is a matter of time (it’s always about that, don’t you think ?).
By publishing software on a public forge, you (implicitly) signed up for dedicating time to others. And it’s what constitutes the power of FOSS. Use it well.
> Post header image was generated using Adobe Firefly