Proposal: Introduce a dev-curfew on open patches

The problem is to define if a river became cleaner.
This is much easier for features design rather than for system design.

Software development is quire representative - it represents skills, demands, knowledge, vision, experience in defining/solving problems and passion of a developer in the same way art represents skills of an artist.

People can solve only problems they are familiar with. We all know only what we deal with.
That mean that you have a better chance to solve a problem you faced during your life, than you faced quite recently - because you has time to figure out possible solutions and make a necessary amount of mistakes before reaching a solution.

To define if a river became cleaner it necessary to
define what exactly is a mud to clean out, otherwise efforts could be put with quite opposite result.

1 Like

Well, the discussion turned into philosophical reasoning.
That’s what I would ask the developers for last.

Pay more attention to old patches, especially those that you have already reviewed and requested some changes, as well as those that contributors update and keep up to date.

If the patch is not interesting, at all or in the near future, say it directly so that the person understands what is happening and does not waste time keeping the patch up to date.

My main complaint about the review process is that it is not always clear what is going on and what to expect. Especially when the patch has been accepted, or the last necessary corrections have been made and then nothing happens for more than a year.

This will help to avoid a situation like the one that happened with @tintwotin.

8 Likes

I think this is a really important one, and it’s something I’ve done in the past (for example D12331: Dope Sheet: Nested action groups). Still, I think such patches are the result of something that is harder to improve: communication beforehand. This also relates to:

The way I see it is that gifting Blender a new feature is like giving somebody a kitten. It’s nice, and cute, and everybody likes it, but it also has to be taken care of, and will take “maintenance”. A patch that adds something new to Blender is similar – not only does it have an impact on the code now, but it will also have to be maintained, fit into the current architecture, and fit into the direction the responsible module wants to take Blender into. “Drive-by contributors” that throw a patch into the tracker and then don’t want to have anything to do with its maintenance over time, or with adjustments to the surrounding code to make things fit better, cannot be accepted lightly. Doing so will add more work onto the plate of already-overloaded developers, and that’s not a healthy way of working.

Giving a gift is fantastic, and my heart warms up when I see how many people want to invest time into making Blender better. But, if there was no communication beforehand, it’s also possible that the contributor is making a cream pie for someone who is lactose intolerant. Just because a lot of effort was put into it doesn’t guarantee that the gift is a perfect fit for the receiving party.

To come back to my earlier point: this is resolved by talking about the approach & software design beforehand with the module members. Present ideas, and if the module likes them, only then actually invest time & effort into writing the actual code. Or pick a task from the already-approved list of ideas; for example, there is a list of TODOs on the workboard of the Animation & Rigging module that are all good, desirable ideas.

21 Likes

I’d like to add that even if an idea is on a pre-approved list, still go talk to the module before starting, just to make sure you’ll implement it in a way that would make it through code review.

5 Likes

@sybren Communication before submitting patches is a good thing, however, this means that there must be active Blender modules with active module owners and active members with time and will to engage in that communication. This is currently not the case in several modules. Communication needs two parties.

3 Likes

I am working on a better module communication. Improve module pages / roles

Discussion is still happening there, but I hope to implement the first improvements in January.

10 Likes

Yes, i can easily find such examples. For example here, code was reviewed and approved. No feedback from appropriate modules after that. Nothing happens for more than a year.
https://developer.blender.org/D9621

3 Likes

I guess the problem in these cases is that there’s nobody who feels qualified to make decisions about that part of blender? Or people who might be qualified are too busy working on other parts?

1 Like

Here is an illustrative patch ⚙ D10929 Cleanup: Fix "QuickTime" spelling, 9 months old.

1 Like

I think that one is (somewhat) on the patch’s author. There are no reviewers added.
If you don’t add reviewers nobody is going to feel responsible for looking at it. Every core developer has enough on his plate as is, people are not actively looking for extra work.

In my experience you need to be pro-active as a patch author as well, just sending it in and leaving it at that is going to get it ignored. There are just not enough core developers to actively keep track of everything that gets sent in.

2 Likes

I think I can take some blame for that one. I can commit spelling fixes without core approval if I just get another native English speaker to confirm first. I probably saw this one, thought I’d deal with it later, and just forgot about it.

The minimum time required for me to commit someone else’s patch is about half an hour. Seems like a long time, but you have to include time to update, apply, compile, test, commit.

Everyone’s time is precious, including mine. Which is why we keep asking you@jenkm – to accept the commit rights we keep offering you so this time is distributed among more people. You are directly responsible for some of our patch approval delays by forcing others to commit your patches for you.

6 Likes

I can commit spelling fixes without core approval if I just get another native English speaker to confirm first.

If the core ones are not against this. @ Blendify is often involved in such as this docs related.

I probably saw this one, thought I’d deal with it later, and just forgot about it.

And I think this is the main reason for the existence of this topic. There are always new patches to check and the old ones are simply forgotten. For me, the old ones should be a priority, so it looks more fair.

Which is why we keep asking you to accept the commit rights we keep offering you so …

It’s not something I don’t want to do, I’m lazy or I’m being naughty. It’s not just that you wrote a commit message and clicked a send button. I’m just not able to do it. I tried to read about it and figure out how git works, for me it’s just a parallel universe.

Moreover, I believe that you should not even offer me the commit rights, since I do not meet the main requirement - understanding the code and what I do with it. Even if it doesn’t look like it from the outside, I don’t really know the basic things.

Yes, but it can also be almost anyone. It is more a matter of finding that time and motivation. I don’t have unlimited time to devote to Blender, so it is difficult to stop actual programming in order to change the word “Quicktime” to “QuickTime” no matter how important you feel that is.

I guess that is where I have a fundamental disagreement. I am far less interested in “fairness” than in the efficiency of the organization and in helping further its rate of improvement. The practical result of “dev-curfew on open patches” is that the core paid devs stop doing real and important work to instead review papercuts and spelling mistakes.

Anyone capable of submitting a patch is capable of committing one. Having the rights to do so does not grant magic - you still have to go through the exact same review process by the same people. It is just that you can do your own grunt work at the end of the process.

This thread is about how to deal with our backlog of patches needing review. The simple answer is that the core devs would need more time to do so. Having them deal with grunt work for no reason does not help at all.

5 Likes

That’s not what I meant. I don’t suggest anyone to spend more time than they are already doing now. And I’m not suggesting giving priority to papercuts. On the contrary, I’m talking about the fact that there are old patches that fix serious bugs, with confirmed reports, etc. But these patches are simply not noticed because they are far at the bottom of the list, and new but less important ones are checked instead.

(Well, as it seems to me from the outside.)

2 Likes

But then your “illustrative patch” was the change of the case of a single letter. It would illustrate your point much better to link to patches that are languishing that are fixes to confirmed bug reports.

It was just a comment on the previous one:

I guess the problem in these cases is that there’s nobody who feels qualified to make decisions about that part of blender?

I gave an example of a patch for which almost everyone is qualified enough. And it will take 5 seconds for three clicks to accept it. But it took almost nine months.

But I think it’s quite fair to reject such patches as not important enough. Just need to clarify it, so that it is clear prior to uploading. (I know it and upload just one percent of small fixes what I would like, there is no problem with that.)

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.

4 Likes

This is better than having a decision made by someone who feels qualified without being qualified.
Nothing is better than damage.

1 Like

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.”

4 Likes

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.

7 Likes