I have been trying to upstream patches to kubernetes and etcd for about a year and ended up giving up. It is impossible to get someone from the project to review my PRs, and since I cannot get PRs under my belt I can not become a maintainer either.
My suspicion is that you get ghosted if you don’t have a @google or @redhat email address and really the only way to become a contributor is to be buddies with someone who works on the project already.
I have considered going to one of the CNCF committee meetings and being like, hey you guys are not accepting new contributions which goes against your mandate. But in the end I just maintain local patches that don’t get upstreamed which is easier.
I haven't seen your PRs and I don't work on those project. I have small projects that receive few patches.
My experience of the few patches I have received though is they are 100% without exception, bad patches. Bad in that, without me putting an hour or 2 of work into them I can't just accept them. The most common case is no tests. The patch fixes an issue, but the issue exists because there was no test for the case the patch is fixing. So, to accept the PR, I have to download it and spend time writing a test.
Other common experiences are bad coding practices and non-matching styles so I have two choices
(1) spend 30-60 minutes downloading the patch, fixing these issues myself
(2) spend 40-60 minutes adding comments to try to get the person who posted the PR to make their patch acceptable (40-60 mins includes the back and forth).
More often than not, (2) never gets a response. The contributor's POV is they provided a fix and I should be happy to take it as is. I get that. At a certain level they are correct. But, these projects are hobby projects and I have limited time. So I generally don't do (2) because if they ignore the comments then it's wasted time, and (1) has the hurdle that I need to take an hour out to deal with it.
Your first example should be solved by the maintainers outlining clear contribution guidelines. It’s not hard to point some automation at a pr and comment if someone didn’t follow contribution guidelines.
Nonmatching styles can be mostly solved with linting and static analysis
There’s no fix for bad code outside of manual review beyond that. But doing those things should significantly cut down on your noise.
I don’t think there is a simple “fits-all” solution.
In my case there is a monetised proprietary “enterprise” edition of the projects available.
Contributions only get accepted if they fit into the commercial roadmap, which is shaped by the (paying) customer needs.
It’s not perfect but the OSS “community” edition is still usable and valuable to many
Thanks for taking the time to reply. I agree that there's no "fits-all" solution & I think that's a good sign of the open-source ecosystem's diversity.
I haven't seen static analyis cover the things I'm concerned with.
Examples, calculating something twice instead of pulling the calculation out of the loop (one case) or into a separate function so that 2 separated places where it's calculated don't get out of sync (a different case). Another might be using an let x; if (cond) x = v1 else x = v2 (which is 3-9 lines depending on your brace style) vs const x = cond ? v1 : v2. When v1 and v2 are relatively simple expressons. I haven't seen a checker that will find stuff like this.
If just close pr without explanation. In contributing guidelines I’d mention that low quality prs will be closed as they waste time and "why was it closed without explanation?" won’t be answered either because it would waste time and the whole point is not to.
It’s not worth the time. You will spend uncountable hours of (unpaid) extremely exhausting labour talking to people who only care about solving their personal super specific problems. This is true for 90%+, there are exceptions but they are exceedingly rare
Trust me I tried many many times.
This has nothing to do with Google being evil it’s just one of the realities of maintaining a big open source project.
I'll add that for small projects, (and I suppose large ones) it's also a "unwelcome" task. Kinda like docs is.
Open Source projects are typically done by people who like coding. Writing docs, reviewing PRs, "management" are all chores, not fun parts of the project.
I manage a couple of projects that get submissions. Handling that is really not the fun part of my day. Fortunately I get very few. I can understand why ones that get a lot see it as a burden, not as the great gift the submitter thinks it is.
Personally I don't want to spend hours each day reviewing PRs. That's not what I signed up for.
It isn’t a me problem, I have had no issues upstreaming patches to other projects which have responsive maintainers but etcd and k8s just seem to be straight up mismanaged. I read contributors.md, I make sure all the linters pass, I make sure all the tests pass, I sign off the commits with my real identity to fulfill the CLA.
Yes there is a lot of garbage out there, but for the people who are actually trying to fix issues it is impossible without an insider within the project.
One problem with tests is that every project has different philosophies on how to write and how to organize tests. Others have no way of running them locally because it‘s all CD, and often figuring out how to write and organize tests takes longer than fixing the bug itself. Or the stack has little support for running just one specific test (aka the test you‘re trying to write). Or tests need resources you don’t have.
I say almost exactly the same thing about agent changes, but the impression I get from people heavily using agents is that they are plenty more flexible about what the code looks like than I am.
I am starting to suspect that it is a personal failing of mine to require that all my code looks consistent within a single project.
Well run projects I have contributed to have linters which fail on bad code style. Ask the submitter to make the linter happy before you review the code.
> Well run projects I have contributed to have linters which fail on bad code style. Ask the submitter to make the linter happy before you review the code.
Linters can't catch most things that are not syntax-style; i.e. linters can't catch semantic style.
Here is code CC generated this morning for me:
size_t bvks_tls_read (bvks_tls_t *tls, void *dst, size_t dst_len)
{
int ret = wolfSSL_read (tls->ssl, dst, (int)dst_len);
if (ret > 0)
return (size_t)ret;
if (ret == 0)
return 0;
// ret < 0, error case
int err = wolfSSL_get_error (tls->ssl, ret);
switch (err) {
case WOLFSSL_ERROR_WANT_READ:
case WOLFSSL_ERROR_WANT_WRITE:
errno = EAGAIN;
break;
case WOLFSSL_ERROR_SYSCALL:
// errno already set by the underlying socket call
break;
default:
errno = EPROTO;
break;
}
return (size_t)-1;
}
size_t bvks_tls_write (bvks_tls_t *tls, const void *src, size_t src_len)
{
int ret = wolfSSL_write (tls->ssl, src, (int)src_len);
if (ret > 0)
return (size_t)ret;
if (ret == 0)
return 0;
// ret < 0, error case
int err = wolfSSL_get_error (tls->ssl, ret);
switch (err) {
case WOLFSSL_ERROR_WANT_READ:
case WOLFSSL_ERROR_WANT_WRITE:
errno = EAGAIN;
break;
case WOLFSSL_ERROR_SYSCALL:
// errno already set by the underlying socket call
break;
default:
errno = EPROTO;
break;
}
return (size_t)-1;
}
One of those is both hard to maintain and has precision (and potential overflow) bugs.
The other isolates the potentially buggy behaviour, validates it, and ensures that future changes to fix size_t/int precision loss bugs only has to be done in a single spot.
No linter is catching that style. It's more "coding style" than "syntax style".
Kubernetes is such a huge project that there are few reviewers who would feel comfortable signing off an an arbitrary PR in a part of the codebase they are not very familiar with.
It's more like Linux, where you need to find the working group (Kubernetes SIG) who would be a good sponsor for a patch, and they can then assign a good reviewer.
(This is true even if you work for Google or Red Hat)
I think if I were a random Google employee submitting Kubernetes patches at my day job-- i.e. not a project maintainer, but just someone in the K8s org chart-- I'd be kind of annoyed if I got cold-emailed asking me to help merge their patches. I'd probably trash that email and assume it was some kind of scam.
I get that the current system isn't working, but I don't think you should just go emailing random committers, that seems likely to just piss people off to no benefit.
Github suggests reviewers to PR authors based on who's been modifying nearby code recently (ok, I don't know whether that's a general policy, but it happens to me all of the time). And for the past year or so I have been getting tagged to review more and more AI slop from newcomers to the project that we chose to maintain in public. I just immediately nope out of all reviews now if I don't recognize the submitter, because I don't scale enough to be the only actual human involved with understanding the code coming at me. This sucks for the newcomers who actually wrote the patch themselves, but I can't always tell. Put some misspellings in your comments and I'm actually more likely to review it!
> It is impossible to get someone from the project to review my PRs
Sorry to say this, but this is natural. Writing patches is easy. Reviewing them is hard. Writing patches (and getting them accepted, merged) is rewarding and demonstrable (as a form of achievement). Reviewing patches, educating new contributors is sometimes rewarding, sometimes not (it's an investment into humans that sometimes pays off, sometimes doesn't), but mostly not a demonstrable achievement in either case. Therefore there is incentive to contribute, and hardly any incentive to review. This is why reviewers are extremely scarce in all open source projects, and why all sustainable projects optimize for reviewer/maintainer satisfaction, not for contributor satisfaction. As an external contributor, you just don't get to allocate scarce resources financed by some commercial entity with no relation to you.
If you want to become a maintainer, or at least want others to review your stuff, don't start by writing code. Start by reading code, and make attempts at reviewing code for others. Assuming you get good at it, established project members should start appreciating it, and might ask you to implement some stuff, which they could be willing to review for you. You first need to give the real kind of effort before you can take it.
And this is why "open development" is a total myth today. Resource allocation and work (chore) distribution are aspects of reality that completely break the wide-eyed, bushy-tailed "new contributors welcome" PR message. Opening up the source code (under whatever license) is one thing, collaborating with randos is an entirely different thing. Can you plan with them in advance? Do they adhere to your deadlines? Can you rely on them when things break? When there are regressions?
> you get ghosted if you don’t have a @google or @redhat email address and really the only way to become a contributor is to be buddies with someone who works on the project already
Yes, and the way to become buddies is to help them out where they are hurting: in their infinite patch review backlogs. Of course, that means you have to invest a whole lot of seemingly thankless learning, for the long run's sake. You have to become an expert with effectively nothing to show for it in the git history. It's totally fair not wanting to do that. Just understand that a ticket that remains open indefinitely, or an uncalled-for contribution that never gets reviewed and merged, may genuinely be better for the maintainers than taking on yet more responsibility for your one-off code contribution.
> I have considered going to one of the CNCF committee meetings and being like, hey you guys are not accepting new contributions which goes against your mandate
According to the above, I bet that "mandate" is a total fake; a PR move only. It does not reflect the actual interests of the organizations with the $$$, which is why it doesn't get followed.
You are right that those orgs should at least be honest and own up to NOT welcoming newcomers or external contributors.
If getting people to review code is that hard that seems like a problem for our new AI age. AI coding appears to rely on getting people to review a lot code and assumes those people will catch the errors.
From my view a lot of the problems of current AI is that people assume others will review and catch any issues. The manual work is getting pushed around like a hot potato.
> If getting people to review code is that hard that seems like a problem for our new AI age. AI coding appears to rely on getting people to review a lot code and assumes those people will catch the errors.
This is precisely what many of us have been repeating incessantly, for ages now. AI is boosting the wrong part of the ecosystem. It only makes the existent bottlenecks more painful.
Many maintainers get exhausted by code reviews, and recharge themselves through coding. The latter is what AI takes away. It demotes engineers to "AI slop reviewers". I don't know why anybody would sign up for that; IMO it leads to immediate burnout.
> According to the above, I bet that "mandate" is a total fake; a PR move only.
The CNCF is a registered non-profit and they have a legal duty to fulfill their mandate.
Like I said, it isn’t worth my time fighting this, I just keep local patches now. Etcd is such a dead project that my patches almost never have had conflicts with new releases, because nothing actually changes in etcd because they don’t facilitate external contributors.
My suspicion is that you get ghosted if you don’t have a @google or @redhat email address and really the only way to become a contributor is to be buddies with someone who works on the project already.
I have considered going to one of the CNCF committee meetings and being like, hey you guys are not accepting new contributions which goes against your mandate. But in the end I just maintain local patches that don’t get upstreamed which is easier.