Proposal: Introduce a dev-curfew on open patches

Also, with all the new hired devs, the need to onboard new contributors through submitted patches is diminishing, so in the end, I guess, when no contributors are needed to drive any Blender development, the problem will have solved itself. Note to self: why do I even care?

Thatā€™s probably a somewhat dim view of things, codereview has struggled for years, and patches from external devs like adaptive subdiv still made it in, contributors sadly have to be more persistent than iā€™d like to get their review. Could things run smoother if time was allocated for it? without a doubt! is everything doomed? I donā€™t think I quite share your views thereā€¦

7 Likes

Patch submitted: Dec 12 2016. Patch found useful by a BF dev: Feb 3, 2021:
https://developer.blender.org/D2411

10 Likes

Small correction, not a BF dev, but the corporate money moving into Blender:

Iā€™ve got a number of patches committed and a number of patches still waiting review. Some patches have had a partial review and then left in a state of limbo. It certainly pays to be persistent to get a review but I think the whole system could be better. It takes effort to keep patches up to date with master too!

I think the preferred way is to get a feature design approved first then create a patch. This is good when you have a clear idea what it is that you want to develop. Sometimes however, patches are coded out of a whim or as an experiment. In this case, having a good use case for the patch helps sell it.

11 Likes

The current system is clearly not working:
Submitted: Sep 6 2015, 1:52 PM
Dev commits: Oct 6, 2021, 12:22 PM

https://developer.blender.org/D1506

4 Likes

A feature proposal system canā€™t be done in the tracker (no category, not allowed), canā€™t really be done here (mentioned in the header), and canā€™t be done in Rclickselect due to not being able to link to code in the tracker to any kind of gitflow. The Developer Chat could be a system, but itā€™s a live chatā€¦ not a code tracker. It also canā€™t be directly linked to any kind of gitflow.

Observation of the patch review system

This in itself is a key flaw, you canā€™t really make a proposal with a use case and solution then link code to it to get it quickly reviewed. There is no pull request system on the website with a GUI to feature request tasks in the tracker.

You also canā€™t automatically build and download a pull request to test the patch as the submission for patches is via a *.diff file from a web form or worked from console commands with Arc. Web based GUI with Gitflow systems could allow this. Right now submitting code as a commit, then to a branch, then pulled to master or to a dev build for revision, then getting the pull built automaticallyā€¦ isnā€™t that evident till you get very deep in the processā€¦ and indeed be very persistent with this custom pipeline and terminology.

Ideal Solution

That alone could be optimized, how you commit, pull, and allow the public to statistically review and approve code for core devs to approve in an informed way.

Ideas

Maybe even adding a poll system on pull requests would work - so a core dev can see the poll and viola, merge to master - do less debate on UX, do quick code cleanup if necessary, and good to go. Also a core dev can filter pull requests by number of votes in the polls for priority and user ā€œrequestā€. This is Blender after all, where we ā€œblendā€ code from the opensource community. It must be imperative to make it easy to do so.

But right now, I have been lurking here years now, and I still am unfamiliar with how to review and how to do pull requests for patches and I still donā€™t know if there is some kind of official gitflow : where I commit code with my linked task/feature proposal to a branch with the ability to pull from the branch to master or to the dev build for revision when done, or commit again to the same task then pull again after the pull request gets feedback. I have to dig through documentation to use other terminology and another systems all over the place (ei, Arc?)ā€¦ and commit via *.diff uploads on a web form? That must be hard to review, first you gotta download, unpack, then build, then review - or you gotta run console commands to review. You also have to use a system completely seperate from the tracker (a big disconnect from the tracker on the website?) The buildbot is not building patches nor committed code to review directly in the tracker per task, there are no distributables per pull request for revision accessible from the tracker GUI. There are no pull request systems familiar toā€¦ many other gitflow solutions - which is a tab, a list, some filters, and a download link. Pull requests also are listed and filterable.

I think a curfew might help - but I also think the optimization could be done within the code pipeline solution and itā€™s centralized public GUI. (ei, familiar terminology, easier centralized access, less clicks, less console, less digging, more community involvement to help revision so even the users can review and give revision and statistics to make life easier for a core dev and module head):

  • Make it easier for the community to review code by knowing what are the open pull requests and their respective builds online with a GUI.
  • Make it quick to get user feedback with simple voting or form based rating systems per pull.
  • Make it easier to know when something has a pull request using official Gitflow terminology directly in the tracker GUI online on the website.
  • And make patches easy to test by the community with buildbots on every pull request (patch submissions) available to the public for download and testing, right now the only real test is the master builds, or specific branches. But itā€™s not per patch!

I agreeā€¦ Patches grow old fastā€¦ and great productive features get lost often, and this is not good for the users of Blender and their businessesā€¦

The community could be capitalized to provide feedback, yet I still donā€™t know where to download and test a patch for feedback, do I have to build it myself to test? Do I need to run a console command and make a build environment to do it? How do I give feedback? An emoji? A comment? No rating, nor form guided feedback for statistics? How can I review something with high statistics to push it up the queue? How does a patch get revision statistics before going to a core dev so the code review avoids design debate and only focuses on code merge conflict and optimization? Where is the pull request tab and builds download where I can download a compiled version with the patch to test as a non-developer and actual artistic user?

Itā€™s those things that I think hold up the revision process most. Familiar Gitflow workflows and tools are not evident with the Blender tracker website and UX. Unlessā€¦ you dive in as a ā€œdeveloperā€.

This leads me to the TLDR.

Thatā€™s the key issue:
You have to be a developer to review features and patches, when itā€™s the users that could and should review and test features and patches for the core devs or module heads to know where to put priority on revision.

Also, feature requests and solutions should be tracked in the code tracker - not splintered in 4+ different websites or custom addons + their sub-ecosystems.

But just for the record. thanks for readingā€¦ and also thanks for the awesome development btw! Despite it all, the work with Blender has been very useful for many people, thanks for all the code.

9 Likes

Huge part of the priority depends on how technically difficult is to implement certain patch/task. How significant, extensive and possibly breaking would the change be to the code base. You absolutely have to be a developer to be able to make that decision.

Some things, which may seem absolutely trivial to implement to someone who isnā€™t a developer, may be actually very difficult to implement properly, and may even require significant redesign of some of the core architecture of the software, so that, for example, adding similar types of features becomes much easier and faster in the future. A simple user with no programming experience can never make such decision.

I agree that BF developers should be more open to user feedback, especially in the area of UI, because itā€™s the UI what allows users to interact with the code written by the developers, and should have proper, official, central place for feature requests instead of shoddy right click select with extremely poor request sorting and filtering features. But at the same time, priority decisions should be left up to them.

1 Like

moving from phabricator to a self hosted gitlab instance is actually planned, but i donā€™t think itā€™s something actively being worked on

1 Like

Iā€™m late to the party but I just wanted to echo this comment and the others like it.

Iā€™ve seen many very exciting new features submitted to developer.blender.org as patches and watched them bit-rot for years after being ignored because either someone said ā€œThereā€™s a better way we could do thisā€ or ā€œCode isnā€™t fit to be mergedā€ or ā€œWeā€™re not sure if this the kind of workflow we want to add to Blender or notā€ due to some concerns over performance or whatever.

Great stuff that makes me kinda sad to think that itā€™s been sitting on the developer portal so long now that it couldnā€™t be merged at this point, without probably a major rewrite to bring it up to date with the blender codebase of today, so basically the opportunity has come and gone for those features.

Stuff like a total rewrite of Blenderā€™s baking system to allow creating batch jobs of combinations of high and low poly objects, options to output PBR textures, and bake everything in one go. Thatā€™s better than the baking addons which folks are paying good money for. But the baking rework didnā€™t get merged because someone said a node based baking system would be better, thus it sat there and collected dust.

Sure, maybe a node based baking system would be betterā€¦ but as a user? Having ANY kind of upgrade for the existing (and kinda terrible, sorry to say) baking system would have been appreciated. As a result, that baking rewrite we could have had in 2018 didnā€™t happen. And I highly doubt weā€™ll have a baking rewrite until at least 2022, if not longer, since baking seems to be constantly just pushed further and further down the scheduleā€¦ Surely merging an imperfect improvement to baking in 2018 and replacing it with a node based system in 2022 would have been better for users?

Thatā€™s just one example, thereā€™s probably a good dozen or more excellent features that have been left to rot on developer.blender.org, because someone said ā€œwe could do this betterā€, but then didnā€™t, so the patch just sat in limbo.

I would rather see these things merged, even if they were only merged under some kind of ā€œExperimental featureā€ flag, and only enabled with a tickbox in preferences, and structured in a way that keeps them fairly isolated from the rest of the code in Blender. Something is better than nothing.

9 Likes

Iā€™ll probably repeat myself, but Iā€™ll say it anyway. Youā€™re talking about adding new features. There are a lot of reasons why these patches can be ignored. Here everything is clear, I will not discuss it. But there are patches that fix bugs. Isnā€™t this a higher priority? And the most interesting thing. There are approved patches that have not been committed. And there are also patches, for example, with changes literally in one line of code, which takes one minute to check.

Well, all sorts of beautiful ā€œpicturesā€ can be viewed here:
https://metrics.blender.org/d/LvXl59qMz/blender-patches

1 Like

This confuses me a lot:


There seem to be quite a few patches with open status that have been accepted. Shouldnā€™t acceptance of a patch automatically close it?

If some minor changes are still planned for a patch after the acceptance, but the patch is finalized enough state to be accepted already, then there seem to be category exactly for that - changes planned. So I donā€™t get how a patch can be accepted and open at the same time.

2 Likes

I think the accuracy of the ā€œacceptedā€ patch is not that accurate. For example, for a patch that has three reviewers, if one of the reviewers accepts the patch, the patch will be in an ā€œacceptedā€ state even after another reviewer comments or the author updates it.

See the Accuracy section

1 Like

No, it is not that simple.

First, patches (usually) are closed by commiting changes in a patch. It is more or less manual thing. ā€œCommitā€ and ā€œpatchā€ are different things. They can have different title/description, patches can be opened and/or changes after closing while commits are there forever set in stone and can only be ā€œreversedā€ which is basically making another commit with opposite changes. Patches and commits even can have different content - sometimes after approving patch author makes several commits (i.e. one for renaming variables / refactoring and other for functional changes) though itā€™s preferable to split patch into several in those cases to make review easier.

Second, some patches are based on others. Like I mentioned here, sometimes itā€™s better to split patch into several parts. In this case one patch can be approved but unable to land since there is no right code to change - itā€™s not there yet.

There might be other nuances but I think itā€™s enough

Patches sometimes need to be approved by multiple people or developers are waiting for another developer to look at the changes and see if they approve

Well, this is an interesting topic

I think the proper solution is a friendly interface or application so clueless users could build their own blender version with ease.

Not sure how much of a challenge this idea represents or if it has been discussed already.
C plugins would also help i guess?

In my humble opinion, the problem is terrible management and planning, because someone has planned an update to ā€œbakingā€ for version 2.92 or the sculpting ā€œbrush managementā€ in version 2.91.


At the same time, at the beginning of 2019, Jacques showed already working nodes for particles, and Pablo - new sculpting possibilities, which have not yet been fully adopted and introduced. Iā€™m not even talking about the fact that we have been waiting for the small ā€œOnly Renderā€ checkbox (which allows showing only those objects that will be displayed during rendering) since version 2.80.

But we do have dotted lines.

5 Likes

The devs have been pretty clear that they regret not being able to not follow up on these. Blender is growing really fast, and a lot of unknowns get in the way, not to mention the need for more and more developers. Itā€™s not like it is malicious, genuinely, they are doing their best, and wish they could adhere better to those graphs. I forget where, there are so many threads and sites for blender stuff etc, but they are trying to make better schedules that people can more reliably look at.

12 Likes

Most patches are built automatically by Harbormaster. Some portion of them available at Blender Builds - blender.org. Please, make all these builds available for users to download, it would be great if the dl link was somewhere near - e.g. in the right panel on the patchā€™s page. The download counter could be a strong signal for devs to focus their attention on the patch.

1 Like

No thatā€™s a dummy step currently configured to do nothing.

Thatā€™s done via buildbot by people with login access to the same. Infrastructure/BuildBot - Blender Developer Wiki