This post explains the policies used in the OpenSimulator project regarding acceptance/rejection of patches. It starts with a few pointers to the existing documentation, then it explains the major criteria for acceptance/rejection, and it ends with a few pointers to examples of recently accepted and rejected patches.

Existing Documentation

• Coding standards.
  Summary: it’s the C# coding standards plus a couple of extra rules
• Code base overview. (credit: Justin)
  A lot more could be said, but documenting the framework properly is a huge amount of work. More below.
• Submitting patches.
  Summary: small is beautiful. I highlight the following paragraph:
  “Please put only one logical change in a patch at a time. Patches that contain more than one logical change tend to be larger, more complex and hence take more time to be applied. At worse, developers will tend not to look at them because it’s hard to disentangle all the possible effects. Multiple logical changes can be in a patch if they only affect a single feature (like, for example, the feature the patch enables).“

Major Acceptance/Rejection Criteria

The pointers above give the easy reasons for why patches may be rejected. For example, patches that don’t follow the C# coding conventions or that are trying to do many things at the same time will likely be either ignored, rejected or pushed back to the contributors with requests for improvements. But these are easy-to-spot problems. There are other problems that aren’t so easy to spot, but that are even more important: whether or not a patch follows the architectural principles of the project.

This may be a foreign concept to to many, so here is an introduction to software architecture. There are many definitions for what software architecture is. I like Dick Taylor‘s definition:

“A system’s architecture is the set of principal design decisions made during its development and any subsequent evolution”

Given this definition, software architecture is usually implicit in the code and present only in the minds of the developers, unless an explicit effort is made to externalize it, which is very rare in open source projects. That’s why it’s so critical that people who want to make non-trivial contributions to open source projects like OpenSimulator join the developers in their day-to-day conversations in order understand the dynamics of design decisions. Alternatively, they may develop a working relationship with one or more developers. In the case of OpenSimulator, most of the action happens in the IRC (irc.freenode.net #opensim-dev); all core developers are there most of the time.

Before explaining the high-level design decisions of OpenSimulator, let me continue to be academic for a moment and point to a wonderful effort made by a colleague in Delft with the goal of (1) teaching students about software architecture and (2) contribute to open source projects in the process. Please follow this link (it will open in another tab):

Delft Students On Software Architecture 2015

As you can see, it took teams of 3-4 students 10 weeks in order to document each of the projects. I won’t do it in one post. But I can explain the very high-level architectural themes of OpenSimulator:

• The core developers are the major stakeholders. Each one of us has high stakes in the code base. We are not the only stakeholders, obviously, there are many more. The large community of users also has a stake in the project; but they don’t have as much power as the core developers or even as the non-core developers. In OpenSimulator, people who contribute code have more power over its evolution than those who don’t. See for example the Power/Interest grid of Jekyll; OpenSimulator has a similar grid.
• The project is a *framework* packaged together with a default application, SL-ish. It’s really important to understand what software frameworks are, with their hot and frozen spots. Two consequences come from OpenSimulator being a framework, rather than a regular application:
  • Patches that touch frozen spots unnecessarily are likely to be rejected. OpenSim.Region.Framework, OpenSim.Framework.Servers, OpenSim.Server, OpenSim.Services.Interfaces, OpenSim.ApplicationPlugins.RegionModulesController and OpenSim itself are the major frozen spots. Everything else is a hot spot.
  • We encourage the encapsulation of new features in region modules (the plugins of the simulator), which extend/replace hot spots. If that implementation proves difficult, we’d like to know why it is difficult; we’ll gladly change the APIs when they prove to be getting in the way of doing things, but it requires discussion.
• We may or may not take in new plugins into the core distribution, depending on how important we think they are for the stakeholders. When we don’t take them in, we encourage their developers to maintain them and make them available to everyone, either by hosting the code in some open source repository or by distributing binary versions of their plugin.
• The existing plugins in the core distribution reflect a certain level of performance, operations and security that the core developers deem acceptable wrt usability and vulnerabilities. Alternatives to these implementations may change levels of performance and security (up or down), and sometimes those changes have tradeoffs in terms of usability and vulnerability. We may or may not accept those new tradeoffs in core. Patches that increase performance and security without any changes to usability and no additional vulnerabilities are usually very welcome and accepted immediately. Patches that move the levels of usability and exposure to vulnerabilities need a serious assessment. If we deem them unacceptable, we continue to encourage the developers to maintain their own implementations and even make them available to others; we just don’t want to have them in the core distribution.
• We encourage the community to develop and maintain their own plugins. Several core developers have their own alternative implementations of many of the core plugins in their own usage of OpenSimulator, as well as lots of additional features developed as plugins. That’s the whole point of having a framework. The core distribution reflects a certain balance in a large spectrum of implementation options that we believe benefit a large portion of the stakeholders, but possibly not all.

Examples of Accepted and Rejected Patches

Here are some examples of patches that have been recently accepted and rejected. For each, I give a brief reason for the decision:

• 7254. Status: accepted. Reason: very small patch, harmless change to existing Robust plugin.
• 7673. Status: accepted. Reason: small bug fix patch to frozen spot, confirmation of resolution by users.
• 7682. Status: accepted. Reason: very small patch, harmless bug fix to Linden client plugin.
• 7345: Status: accepted after discussion. Reason: very small patch for fixing annoying bug in AttachmentsModule plugin. Issue of discussion: hard-coded sleeps and placement of the code.
• 7676. Status: rejected.  Reason: very small patch, but with unacceptable downtime for existing grids.
• 7677. Status: rejected. Reason: very small patch, but with unacceptable change in security of one of the Hypergrid plugins vs. usability.
• 7653. Status: rejected as is, waiting for improvement. Reason: very large patch trying to do too many things at the same time, and with unnecessary intrusion in frozen spots of the framework. Part of it should be rewritten as a plugin, other parts could also be rewritten as a (different) plugin.