In a nutshell, if the ambition is to keep Blender as an open-source project which embraces contributions, then this is currently colliding with hired devs, who want to spend their time on their own work.
To solve this, the leadership can either shut down the open invitation for contributions or kindly ask the hired devs to allocate the time needed to actively deal with the contributions in an organized way.
The bottom line of this thread is that you can’t have both. You can’t ask for contributions and not have anyone to deal with them.
That one-character spelling patch you referenced took me 45 minutes to commit in total - it was part of an optional component so I had to do a full build. So, as mentioned, this isn’t about “why can’t someone spend 5 seconds to click something” but instead “why can’t a volunteer find 45 minutes to do this particular thing versus all the other things they could be doing to improve blender.”
Does that include the ~10 minutes to run the tests? The number of times things got committed in a rush without testing going “this shouldn’t break anything” yet breaking some tests, is much further from zero than i’d like it to be.
I don’t think the problem is ‘hired devs, who want to spend their time on their own work’. Everybody wants to spend time on their own work, not only the hired devs. The problem is just that there are more people sending in patches than there are people available with enough overview of the complete codebase to be able commit them.
This is something all open source projects struggle with. In my experience blender does better than most. It’s far from perfect, but still better than most.
That’s not to say we shouldn’t keep searching for systems to help us manage this stuff better. I just don’t think there’s a silver bullet which can actually fix it completely.
One solution would be to ‘not accept contributions’. But that would hardly be productive, because lots of progress blender makes does come from contributions, even if a lot of stuff falls through the cracks.
Another solution might be to hire someone specifically to just commit external contributions. It would need to be someone who gets intimately familiar with the whole codebase, then, because contributions are all over the place. Not impossible, but not easy either. And once someone gets to that level (s)he is probably more productive to blender as a whole doing something else.
If you are closer to the codebase and deep code handling (math pole), you are farther from its actual application (art pole) and vice versa. This happens because you have limited time which allows you to be good in some specific area. Getting full stack is costly in development, getting wide range is costly in production.
I tried to depict such a dependency here.
Switching between tasks take efforts, getting deep into a task (obtaining a full context) take efforts.
In my opinion, prioritizing the tasks (task ranking) is also a separate management job that connects devs and industry requirements, so you have to be familiar with both of them at quite deep level, to distribute the amount of overall care properly.
But managing wide range require mastering wide range first.
In this position, you will have too much power and the ability to screw up a lot of things if you are not qualified in some areas (for example, when an animator familiar with maya is trying to design modeling workflows, confusing system design with features design or usability with low entry threshold), so this power is distributed among the modules.
There are a couple of things a new hire could in fact do with the patch tracker, such as making sure the code compiles properly and fits the style guide. However, to actually commit the patch requires the guidance of someone who knows how the code works and how the new code will interact with existing code (ie. making sure the patch for starters is not a hack that could lead to a long game of whack-a-mole bugfixing). Please note the devs. look at the long-term as well, and while some patches look great from a user’s point of view, it could have broken design that could lead to a lot of dev. time on refactors that are preventable.
Committing blind is never recommended, and a hire who actually commits patches for the purpose of getting them in will do just that.
I think there are some improvements that could be imagined to the build/phab infrastructure, for instance the straight forward Quicktime → QuickTime rename, most devs will be perfectly capable of going “yeah this looks fine” in well under a minute.
If there was functionality in our build infra to go “Hey I sign off on this, if this builds, and passes all tests on all platforms, just go ahead and land this for me” I’d use it, there’s no reason I should be spending 30-45 minutes on that, it’s something that could be the job of some automated process.
Isn’t testing part of the review process? If you have accepted the patch, it means that it is correct, safe and can be committed. (If there is no additional comment.)
And running tests is definitely not for a contributor. You just can’t rely on “some kind of” contributor to do everything right. I’ve seen your recent discussions about how someone with commit rights did something wrong and no one noticed it for years.
Hah, I wish. “Being reviewed by a human” is something completely different from “being bug-free”. Otherwise we’d be reviewing even more, and the bug tracker would be empty.
First of all, my comment was in the context of simple diffs, like the one I mentioned that corrected a spelling mistake, reviewing that vs reviewing a brand new cycles feature quite a different review process. One I can look at an go, “yeah this looks fine”, the other deserves a much much closer look.
Second we run the tests every time the buildbot builds for a reason, it’s virtually impossible to make sure a single innocent looking change does not break any of our major supported platforms in unintended ways. I’ve had commits, that I had reviewed, I made sure all tests passed on my windows machine and they still caused a build or test error for either mac and/or Linux.
I’m going to politely disagree there, I think it most definitely is, it’s your change, it’s your job to make sure your fix or feature didn’t break anything. If I’m reviewing your patch and it broke a test, it will be returned to you with a message saying “this breaks test ABC, please fix”
But you can rely on reviewers who are scrambling for time due to their existing workload to do “every thing right” ? They are human, they will make mistakes.
Reviewers ought to spend their relative precious time looking at the guts of the diff, what is the diff doing, is it following our code style, are they using a bad algorithm, is it fitting in the overall blender architecture, etc. Spending 30-40 minutes doing a full build of blender and running the unit tests for a spelling change is a waste of time, robots can do it (Point is our current bots already can, they just lack the landing phase I envisioned)
What I have written is just my experience in two years. All are glad to see a little polishing of the interface, but do not have the ability to spend time on it, since there is always something more important.
We have already spent more time on these comments than it would take to make the proposed changes. So, IMHO, if such (not requiring a separate discussion) changes can be made at the same time with others, then why not.
And again, I just see that such patches can wait more than a year for at least some reaction from the developers, although they look straightforward.
And the problem is that there is no other way to make such minor changes. Although no one is against such improvements.
Such a thing cannot go through the normal review process since developers are not interested in it as these are too insignificant.
The only possibility here is incorporation into a larger patch.
We have a team of developers who are already spread too thin over many different areas of Blender. The solution is not to make existing patches more complex than they have to be.
Also please do not read a patch sitting in the tracker as disinterest from developers. It’s safe to assume that pretty much all developers are interested in improvements to Blender, big or small. It’s more a matter of having the time and the mental space to do yet another context switch.
A patch appearing straight-forward does not mean it costs zero energy to context-switch to it, to read through the patch description, to figure out the writer’s mental model when they were writing it (necessary because often details are ommitted), then to decide whether the patch is actually doing anything desirable, and to check that it doesn’t have any unintended/hidden side-effects.
This is exactly why I suggested including the changes in that patch rather than making a separate one. It’s just, IMHO, easier for you. But of course you better know what’s best for you.
And of course I will not add a separate patch for this because there are (and mine among them) more important fixes that are waiting for review for a long time.
Thanks for the offer to help @cata_cg. Anyone is welcome to help in reviews. Especially when it comes to the “detail level”, like code style, naming conventions, adequate code comments, etc. it’s not that hard to just join in and help out. Also when it’s about the local design of the code, i.e. the design of the patch itself, I think it’s fine to help out; just as an example, if you see that something is quadratic in complexity when it could also be linear, feel free to offer such feedback.
When it comes to bigger code design aspects, like whether the design of the patch actually matches the overall design of Blender, then of course it helps to have more Blender development experience/knowledge.
Thanks for the note. Moved the comments.