This obviously wouldn’t replace the need for proper documentation, and there are a lot of things it wouldn’t be applicable to. But I think it would be a really valuable supplement for certain parts of Blender’s code base.
A handful of examples that would have been helpful to me and/or the animation & rigging module in the past:
Adding a node to the compositing nodes.
Adding a tree view somewhere in the UI.
Adding a new ID type.
Adding a new dependency relationship in the depsgraph.
The main benefits of these kinds of things being in the form of a PR are:
Since they’re something that actually builds and works, important details that might seem obvious to a seasoned dev and thus get omitted in normal documentation don’t accidentally get left out.
They serve as a good, practical overview of what actually needs to be changed to accomplish something. Kind of a “living tutorial”.
When they get out of date, they can be updated like any other PR, with the confidence that they again build and work, and that they accurately represent what’s newly needed to accomplish the task.
With e.g. “adding a new ID type”, it gives an overview of all the moving parts necessarily involved in an ID, which can be helpful as reference even when working with existing IDs.
To be clear: I’m not suggesting that anyone be obligated to make or maintain example PRs. Rather, I would just like the option for developers to create and maintain them as a form of documentation when it makes sense and is practical.
Having a label that clearly identifies them would be helpful, so that people can both filter for them when looking for an example and also filter them out when not.
Maintenance is gonna be the main problem there, as you noted, they are “something that actually builds and works” but that claim only holds true for when the PR was last updated, as the codebase moves fast, PR’s can get stale rapidly, sometimes hours after creating it something comes along that breaks them.
and while you mention they can be updated, the answer of who is going to be doing that and how they will be notified when things will inevitably break, is seemingly a problem left for the reader.
Personally, keeping PR’s that have a chance to land in main updated during the review process is annoying enough, having a PR that’ll remain open till the end of time, and i’ll have to maintain, and it’ll never actually land in main… yeah… no…that be a lager assault on my available time than i’m willing to give up.
Don’t get me wrong, I’m not disagreeing with the potential benefits, it’s just the costs are hard to justify.
tagging some PR’s as hey this one has good stuff in it, so you don’t have to wade though a whole bunch of commits that fix spelling errors, sure, that certainly can be done, the upkeep of them i’m less convinced about.
To be clear: I’m not suggesting that anyone be obligated to make or maintain example PRs.
In other words, if it’s not something you have the time to do, that’s fine! Indeed, some parts of Blender’s code base move very fast, and it probably isn’t worth the effort there.
Instead we’d link the example PR, with the type of disclaimer I suggested there. Saying it was valid at the time of writing, but that you additionally need to find a similar ID type and compare how it’s integrated.
To my ears, the “Good” makes it sound like a real PR that happens to contain good stuff, rather than an intentionally curated example PR. (Although perhaps it would be good to have a label for that as well.)
It may be more discoverable to have a single label, so you can click it and see the full list? You don’t know ahead of time which type of example is available.
They will also be separate by Open and Closed status.
That’s a fair point. It would be nice to have one label for both.
On the other hand, that would make it hard to filter out the PRs that are just examples. Not sure how important that is in practice, though.
I think it’s also worth considering what the primary purpose of the label is. Even if it’s sometimes used to mark existing real PRs that happen to double as good examples, I wasn’t thinking that would be its primary purpose. After a real PR has landed, it isn’t updated/maintained anymore, so the life expectancy of usefulness is limited. Of course, we can just remove the label when that happens.
(In the end, I also just aesthetically prefer the shorter, more succinct label. But that’s not a strong argument in its favor.)