Code contributions to OSL are always welcome. That's a big part of why it's an open source project. Please review this document to get a briefing on our process.
Contributors should be reading the osl-dev mail list:
You can sign up for the mail list on your own using the link above.
The ASWF Slack has an openshadinglanguage channel.
Sign up for the Slack on your own, then under "channels", select "browse
channels" and you should see the openshadinglanguage channel (among those of
the other projects and working groups).
We use GitHub's issue tracking system for reporting bugs and requesting enhancements: https://github.com/AcademySoftwareFoundation/OpenShadingLanguage/issues
If you are merely asking a question ("how do I..."), please do not file an issue, but instead ask the question on the OSL developer mail list.
If you are submitting a bug report, please be sure to note which version of OSL you are using, on what platform (OS/version, which compiler you used, and any special build flags or other unusual environmental issues). Please give an account of
- what you tried
- what happened
- what you expected to happen instead
with enough detail that others can reproduce the problem.
To protect the project -- and the contributors! -- we do require a Contributor License Agreement (CLA) for anybody submitting changes.
-
Corporate CLA : If you are writing the code as part of your job, or if there is any possibility that your employers might think they own any intellectual property you create. This needs to be executed by someone who has signatory power for the company.
-
Individual CLA : If you are an individual writing the code on your own time, using your own equipment, and you're SURE you are the sole owner of any intellectual property you contribute.
The easiest way to sign CLAs is digitally using EasyCLA. Companies who prefer not to use the online tools may sign, scan, and email the executed copy to [email protected].
The CLA allows a company to name a "CLA Manager" (who does not need signatory power) who has the ability to use the online system to add or delete individual employees of the company who are authorized to submit pull requests, without needing to get an executive to amend and sign the agreement each time.
Please note that these CLAs are based on the Apache 2.0 CLAs, and differ minimally, only as much as was required to correctly describe the EasyCLA process and our use of a CLA manager.
Contribution sign off
ASWF requires the use of the Developer’s Certificate of Origin 1.1 (DCO), which is the same mechanism that the Linux® Kernel and many other communities use to manage code contributions. The DCO is considered one of the simplest tools for sign offs from contributors as the representations are meant to be easy to read and indicating signoff is done as a part of the commit message.
Here is an example Signed-off-by line, which indicates that the submitter accepts the DCO:
Signed-off-by: John Doe <[email protected]>
You can include this automatically when you commit a change to your local
git repository using git commit -s. You might also want to
leverage this command line tool for
automatically adding the signoff message on commits.
The best way to submit changes is via GitHub Pull Request. GitHub has a Pull Request Howto.
All code must be formally reviewed before being merged into the official repository. The protocol is like this:
-
Get a GitHub account, fork AcademySoftwareFoundation/OpenShadingLanguage to create your own repository on GitHub, and then clone it to get a repository on your local machine.
-
Edit, compile, and test your changes. Run clang-format (see the instructions on coding style below).
-
Push your changes to your fork (each unrelated pull request to a separate "topic branch", please).
-
Make a "pull request" on GitHub for your patch.
-
If your patch will induce a major compatibility break, or has a design component that deserves extended discussion or debate among the wider OSL community, then it may be prudent to email osl-dev pointing everybody to the pull request URL and discussing any issues you think are important.
-
All pull requests automatically launch CI jobs on GitHub Actions to ensure that the build completes and that the tests suite runs correctly, for a variety of platform, compiler, library, and flag combinations. The status of the CI tests for your PR will be displayed on the GitHub PR page. We will not accept PRs that don't build cleanly or pass the existing testsuite.
-
The reviewer will look over the code and critique on the "comments" area, or discuss in email. Reviewers may ask for changes, explain problems they found, congratulate the author on a clever solution, etc. But until somebody says "LGTM" (looks good to me), the code should not be committed. Sometimes this takes a few rounds of give and take. Please don't take it hard if your first try is not accepted. It happens to all of us.
-
After approval, one of the senior developers (with commit approval to the official main repository) will merge your fixes into the master branch.
C++ implementation should be named *.cpp. Headers should be named .h.
All headers should contain
#pragma once
All source files should begin with these three lines:
// Copyright Contributors to the Open Shading Language project.
// SPDX-License-Identifier: BSD-3-Clause
// https://github.com/AcademySoftwareFoundation/OpenShadingLanguage
as a comment in the syntax of whatever source code is used in that file (for
example, for python or shell files, use # rather than //).
Occasionally a file may contain substantial code from another project and will also list its original copyright and license information. Do NOT alter that notice or copy it to any new files, it really only applies to the particular file in which it appears.
We use clang-format to uniformly format our source code prior to PR submission. Make sure that clang-format is installed on your local machine, and just run
make clang-format
and it will automatically reformat your code according to the configuration
file found in the .clang-format file at the root directory of the OIIO
source code checkout.
One of the CI test matrix entries runs clang-format and fails if any
diffs were generated (that is, if any of your code did not 100% conform to
the .clang-format formatting configuration). If it fails, clicking on that
test log will show you the diffs generated, so that you can easily correct
it on your end and update the PR with the formatting fixes.
If you don't have clang-format set up on your machine, and your patch is not very long, you may find that it's more convenient to just submit it and hope for the best, and if it doesn't pass the CI test, look at the diffs in the log for the "clang-format" CI run and make the corrections by hand and then submit an update to the patch (i.e. relying on CI to run clang-format for you).
Because the basic formatting is automated by clang-format, we won't enumerate the rules here.
For the occasional non-clang-format regions of code, NEVER alter somebody else's code to reformat just because you found something that violates the rules. Let the group/author/leader know, and resist the temptation to change it yourself.
Each line of text in your code, including comments, should be at most 80 characters long. Exceptions are allowed for those rare cases where letting a line be longer (and wrapping on an 80-character window) is actually a better and clearer alternative than trying to split it into two lines. Sometimes this happens, but it's extremely rare.
We prefer three (3) consecutive empty lines between freestanding functions or class definitions, one blank line between method declarations within a class definition. Put a single blank line within a function if it helps to visually separate different sequential tasks, but don't put multiple blank lines in a row within a function, or blank lines right after an opening brace or right before a closing brace. The goal is to use just enough white space to help developers visually parse the code (for example, spotting at a glance where new functions begin), but not so much as to spread it out or be confusing.
In general, classes and templates should start with upper case and
capitalize new words: class CustomerList; In general, local variables
should start with lower case. Macros should be ALL_CAPS, if used at all.
If your class is extremely similar to, or modeled after, something in the standard library, Boost, or something else we interoperate with, it's ok to use their naming conventions. For example, very general utility classes and templates (the kind of thing you would normally find in std or boost) should be lower case with underscores separating words, as they would be if they were standards.
template <class T> shared_ptr;
class scoped_mutex;
Names of data should generally be nouns. Functions/methods are trickier: a the name of a function that returns a value but has no side effects should be a noun, but a procedure that performs an action should be a verb.
Try to avoid public data members, although there are some classes that serve a role similar to a simple C struct -- a very straightforward collection of data members. In these, it's fine to make the data members public and have clients set and get them directly.
Private member data should be named m_foo (alternately, it's ok to use the common practice of member data foo_, but don't mix the conventions within a class).
Private member data that needs public accessors should use the convention:
void foo (const T& newfoo) { m_foo = newfoo; }
const T& foo () const { return m_foo; }
Avoid multiple inheritance.
Namespaces: yes, use them!
Prefer C++11 std rather than Boost or other third party libraries, where
both can do the same task.
If you see a third party library already used as a dependency (such as OIIO, Boost, Imath, or LLVM), feel free to any of its public features in OSL internals (provided those features are present in the minimum version of that library that we support).
Please do not add any NEW dependencies without debate on osl-dev and approval of the project leader.
Use these libraries for OSL internals, but please DO NOT let them infect any of our public APIs unless it's been thoroughly discussed and approved by the group. (Exceptions: it's ok to use OIIO and Imath classes in our public APIs.)
Comment philosophy: try to be clear, try to help teach the reader what's going on in your code.
Prefer C++ comments (starting line with //) rather than C comments (/* ... */).
For any function that may be used by other programmers (e.g., public or protected members of classes), please use Doxygen-style comments. They looks like this:
/// Explanation of a class. Note THREE slashes!
class myclass {
....
float foo; ///< Doxygen comments on same line look like this
}
If you know Doxygen well, please feel free to use the various other markups. But don't go so crazy with Doxygen markups that the comment itself, in an ordinary editor, is not as readable as possible. The Doxygen-generated pages are nice, but the place that needs to be most readable is in the code.
Macros should be used only rarely -- prefer inline functions, templates, enums, or "const" values wherever possible.
When in doubt, look elsewhere in the code base for examples of similar structures and try to format your code in the same manner.