Skip to content

Commit 84b696d

Browse files
authored
docs: AI contribution policy and other docs touch-ups (#2099)
We've discussed AI code assistant policy in multiple TSC meetings, and on the OpenImageIO slack (technically it was a shared OIIO+OSL discussion, but we had to pick a single place to have it). We iterated on the policy document as an OIIO PR, so this PR here in OSL land is just checking that same document into our repo. * Duplicate the OIIO document. We will keep them in sync until such time as the needs of the projects diverge or their communities have different ideas about what policies to articulate. * Appropriate references/pointers in supporting documents: README, CONTRIBUTING, and PULL_REQUEST_TEMPLAGE. I took the time to diff some of those documents and port other recent changes from the OIIO side to OSL. These are mostly minor fixes or improvements in wording, nothing substantive. Signed-off-by: Larry Gritz <lg@larrygritz.com>
1 parent f2cd0ff commit 84b696d

4 files changed

Lines changed: 377 additions & 40 deletions

File tree

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,46 @@
11

2-
## Description
2+
3+
4+
------ :scissors: -------------------------------------------------------------------
5+
6+
YOU MAY DELETE ALL OF THIS IF YOU ALREADY HAVE A DESCRIPTIVE COMMIT MESSAGE!
7+
8+
This is just a template and set of reminders about what constitutes a good PR.
9+
But please look over the checklist at the bottom.
10+
11+
If THIS TEXT is still in your PR description, we'll know you didn't read the
12+
instructions!
13+
14+
15+
16+
17+
### Description
318

419
<!-- Please provide a description of what this PR is meant to fix, and -->
520
<!-- how it works (if it's not going to be very clear from the code). -->
621

7-
## Tests
22+
### Tests
823

924
<!-- Did you / should you add a testsuite case (new test, or add to an -->
1025
<!-- existing test) to verify that this works? -->
1126

1227

13-
## Checklist:
28+
### Checklist:
1429

1530
<!-- Put an 'x' in the boxes as you complete the checklist items -->
1631

17-
- [ ] I have read the [contribution guidelines](../CONTRIBUTING.md).
18-
- [ ] I have updated the documentation, if applicable.
19-
- [ ] I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
20-
- [ ] My code follows the prevailing code style of this project. If I haven't
21-
already run clang-format v17 before submitting, I definitely will look at
22-
the CI test that runs clang-format and fix anything that it highlights as
23-
being nonconforming.
32+
- [ ] **I have read the guidelines** on [contributions](https://github.com/AcademySoftwareFoundation/OpenShadingLanguage/blob/main/CONTRIBUTING.md) and [code review procedures](https://github.com/AcademySoftwareFoundation/OpenShadingLanguage/blob/main/docs/dev/CodeReview.md).
33+
- [ ] **I have read the [Policy on AI Coding Assistants](https://github.com/AcademySoftwareFoundation/OpenShadingLanguage/blob/main/doc/dev/AI_Policy.md)**
34+
and if I used AI coding assistants, I have an `Assisted-by: TOOL / MODEL`
35+
line in the pull request description above.
36+
- [ ] **I have updated the documentation** if my PR adds features or changes
37+
behavior.
38+
- [ ] **I am sure that this PR's changes are tested in the testsuite**.
39+
- [ ] **I have run and passed the testsuite in CI** *before* submitting the
40+
PR, by pushing the changes to my fork and seeing that the automated CI
41+
passed there. (Exceptions: If most tests pass and you can't figure out why
42+
the remaining ones fail, it's ok to submit the PR and ask for help. Or if
43+
any failures seem entirely unrelated to your change; sometimes things break
44+
on the GitHub runners.)
45+
- [ ] **My code follows the prevailing code style of this project** and I
46+
fixed any problems reported by the clang-format CI test.

CONTRIBUTING.md

Lines changed: 156 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,26 @@ why it's an open source project. Please review this document to get a
1010
briefing on our process.
1111

1212

13+
General Tips for Open Source Development
14+
----------------------------------------
15+
16+
* GitHub's [Open Source Guides](https://opensource.guide/)
17+
- Especially the guide on [How to Contribute to Open Source](https://opensource.guide/how-to-contribute/)
18+
19+
1320
Mail List and Slack
1421
-------------------
1522

1623
Contributors should be reading the osl-dev mail list:
1724

1825
* [osl-dev](https://lists.aswf.io/g/osl-dev)
1926

20-
You can sign up for the mail list on your own using the link above.
27+
You can sign up for the mail list on your own using the link above.
2128

22-
The [ASWF Slack](https://slack.aswf.io/) has an `openshadinglanguage` channel.
23-
Sign up for the Slack on your own, then under "channels", select "browse
24-
channels" and you should see the openshadinglanguage channel (among those of
25-
the other projects and working groups).
29+
* The [ASWF Slack](https://slack.aswf.io/) has an `openshadinglanguage`
30+
channel. Sign up for the Slack on your own, then under "channels", select
31+
"browse channels" and you should see the openshadinglanguage channel (among
32+
those of the other projects and working groups).
2633

2734

2835
Bug Reports and Issue Tracking
@@ -34,25 +41,56 @@ https://github.com/AcademySoftwareFoundation/OpenShadingLanguage/issues
3441

3542
**If you are merely asking a question ("how do I...")**, please do not file an
3643
issue, but instead ask the question on the [OSL developer mail
37-
list](https://lists.aswf.io/g/osl-dev).
44+
list](https://lists.aswf.io/g/osl-dev) or on the Slack channel.
3845

3946
If you are submitting a bug report, please be sure to note which version of
4047
OSL you are using, on what platform (OS/version, which compiler you used,
4148
and any special build flags or other unusual environmental issues). Please
42-
give an account of
49+
give a specific, as-detailed-as-possible account of
50+
51+
* what you tried (command line, source code example)
52+
* what you expected to happen
53+
* what actually happened (crash? error message? ran but didn't give the
54+
correct result?)
55+
56+
with enough detail that others can easily reproduce the problem just by
57+
following your instructions. Please quote the exact error message you
58+
received. If you are having trouble building, please post the full cmake
59+
output of a fresh VERBOSE=1 build.
60+
61+
Suspected security vulnerabilities should be reported by the same process.
62+
If confidentiality precludes a public question or issue for any reason, you
63+
may contact us privately at [security@openimageio.org](security@openimageio.org).
64+
65+
66+
Policy on AI Tools
67+
------------------
4368

44-
* what you tried
45-
* what happened
46-
* what you expected to happen instead
69+
Please read our [Policy on AI Coding Assistants](docs/dev/AI_Policy.md)
70+
before contributing or particpating in the project in any way mediated by "AI"
71+
assistants.
4772

48-
with enough detail that others can reproduce the problem.
73+
High-level summary:
74+
- Human must always be in the loop, and is the responsible party for
75+
the contents of a PR (including fully understanding and being able
76+
to explain, defend, and modify it in response to review comments).
77+
- Interact with the project and community yourself, not by agent.
78+
- Disclose what tools you used and how. At a minimum, we require an
79+
"Assisted-by: TOOL/MODEL" line in the commit comments and PR description.
80+
- Don't waste maintainer's time with low quality PRs.
81+
82+
Please do read the whole [Policy on AI Coding Assistants](docs/dev/AI_Policy.md)
83+
for all the details.
4984

5085

5186
Contributor License Agreement (CLA) and Intellectual Property
5287
-------------------------------------------------------------
5388

54-
To protect the project -- and the contributors! -- we do require a
55-
Contributor License Agreement (CLA) for anybody submitting changes.
89+
To protect the project -- and the contributors! -- we do require a Contributor
90+
License Agreement (CLA) for anybody submitting changes. This is for your own
91+
safety, as it prevents any possible future disputes between code authors and
92+
their employers or anyone else who might think they might own the IP output of
93+
the author.
5694

5795
* [Corporate CLA](https://github.com/AcademySoftwareFoundation/OpenShadingLanguage/blob/main/ASWF/CLA-corporate.md) :
5896
If you are writing the code as part of your job, or if there is any
@@ -65,15 +103,28 @@ Contributor License Agreement (CLA) for anybody submitting changes.
65103
equipment, and you're SURE you are the sole owner of any intellectual
66104
property you contribute.
67105

68-
The easiest way to sign CLAs is digitally [using EasyCLA](https://corporate.v1.easycla.lfx.linuxfoundation.org).
106+
Please note that it is extremely common (nearly ubiquitous in the US and
107+
Canada, and maybe other places) for full-time employees of technology and
108+
entertainment companies to have IP clauses in their employment agreement (in
109+
that pile of paperwork you sign on your first day of work and then promptly
110+
forget about) that give the company rights to any code you write, even on your
111+
own time. The Open Shading Language project expects you to know and follow the
112+
rules of your employer and to have them sign a corporate CLA if necessary.
113+
114+
The easiest way to sign CLAs is digitally [using
115+
EasyCLA](https://corporate.v1.easycla.lfx.linuxfoundation.org). There are
116+
detailed step-by-step instructions about using the EasyCLA system for
117+
[corporate CLAs](https://docs.linuxfoundation.org/lfx/easycla/v2-current/contributors/corporate-contributor)
118+
and [individual CLAs](https://docs.linuxfoundation.org/lfx/easycla/v2-current/contributors/individual-contributor#github).
119+
69120
Companies who prefer not to use the online tools may sign, scan, and email
70121
the executed copy to manager@lfprojects.org.
71122

72-
The CLA allows a company to name a "CLA Manager" (who does not need
73-
signatory power) who has the ability to use the online system to add or
74-
delete individual employees of the company who are authorized to submit pull
75-
requests, without needing to get an executive to amend and sign the
76-
agreement each time.
123+
The corporate CLA allows a company to name a "CLA Manager" (who does not need
124+
signatory power) who has the ability to use the online system to add or delete
125+
individual employees of the company who are authorized to submit pull
126+
requests, without needing to get an executive to amend and sign the agreement
127+
each time.
77128

78129
Please note that these CLAs are based on the Apache 2.0 CLAs, and differ
79130
minimally, only as much as was required to correctly describe the EasyCLA
@@ -93,14 +144,90 @@ as a part of the commit message.
93144
Here is an example Signed-off-by line, which indicates that the submitter
94145
accepts the DCO:
95146

96-
`Signed-off-by: John Doe <john.doe@example.com>`
147+
Signed-off-by: John Doe <john.doe@example.com>
97148

98149
You can include this automatically when you commit a change to your local
99-
git repository using <code>git commit -s</code>. You might also want to
150+
git repository using `git commit -s`. You might also want to
100151
leverage this [command line tool](https://github.com/coderanger/dco) for
101152
automatically adding the signoff message on commits.
102153

103154

155+
Commit messages
156+
---------------
157+
158+
### Summary heuristic
159+
160+
Look at the commit history of the project to get a sense of the style and
161+
level of detail that is expected in commit messages.
162+
163+
### General guidelines and expectations
164+
165+
The first line of the commit message should be a short (less than 80
166+
characters) summary of the change, prefixed with the general nature of the
167+
change (see below).
168+
169+
The rest of the commit message should be a more detailed explanation of the
170+
changes. Some commits are self-explanatory and don't need more than the
171+
summary line. Others may need a more detailed explanation. Hallmarks of
172+
a good commit message include:
173+
174+
* An explanation of why the change is necessary and what you hope to
175+
accomplish with it.
176+
* A description of any major user- or developer-facing changes that people
177+
should be aware of: changes in APIs or behaviors, new features, etc.
178+
* An explanation of any tricky implementation details or design decisions that
179+
might not be immediately obvious to someone reading the code.
180+
* Guideposts for somebody reviewing the code to understand the rationale and
181+
have any supporting background information to fully understanding the code
182+
changes.
183+
184+
Remember that at some point in the future, a developer unfamiliar with your
185+
change (maybe you, long after you've forgotten the details) might need to
186+
understand or fix your patch. Keep that person in mind as your audience and
187+
strive to write a commit message that explains the context in a way that saves
188+
them time and effort. Be the hero in the story of their future debugging
189+
effort!
190+
191+
### Using "conventional commits" prefixes
192+
193+
We strive to follow the recommendations known as [conventional
194+
commits](https://www.conventionalcommits.org/), which means that we would like
195+
merged commit messages to have their first line start with one of the
196+
following prefixes:
197+
198+
- `feat:` new features
199+
- `fix:` bug fixes
200+
- `perf:` performance improvements
201+
- `api:` changes to the public APIs
202+
- `int:` changes to code internals that don't affect the public API
203+
- `build:` changes to the build system
204+
- `test:` changes to the test suite or unit tests
205+
- `ci:` changes to the CI system
206+
- `docs:` changes to the documentation
207+
- `refactor:` code refactoring
208+
- `style:` formatting or other stylistic non-functional changes to the code
209+
- `admin:` project administration or policy changes
210+
- `revert:` reverting a previous commit
211+
212+
Obviously, some (if not most) PRs may qualify for more than one of these
213+
categories (for example, a new feature may also introduce a new API call, add
214+
tests, and include new documentation). If that is the case, use your best
215+
judgment to pick the category that best captures the essence or most important
216+
aspect of the change. When ambiguous, consider the list above to be a priority
217+
ranking (e.g., a change that fixes a bug and adds a new test should be listed
218+
under `fix:`, because that appears first in the list).
219+
220+
It is also encouraged, when it makes sense to do so, to put a subcategory in
221+
parenthesis after the prefix, like `fix(optix):` or `feat(oslc):`. If there
222+
is no clear single format or class that is the man focus of the patch, then
223+
you can omit the subcategory.
224+
225+
API or ABI-breaking changes should additionally be marked with an exclamation
226+
point at the end of the prefix, like `feat!:` or `api!:` to make it easily
227+
identifiable as a breaking change from the first line of the commit message.
228+
229+
230+
104231
Pull Requests and Code Review
105232
-----------------------------
106233

@@ -114,7 +241,7 @@ repository. The protocol is like this:
114241
own repository on GitHub, and then clone it to get a repository on your
115242
local machine.
116243

117-
2. Edit, compile, and test your changes. Run clang-format (see the
244+
2. Edit, compile, and test your changes. Run clang-format (see the
118245
instructions on coding style below). Our current formatting standard,
119246
as checked by our CI, uses clang-format 17.0.
120247

@@ -142,7 +269,10 @@ this takes a few rounds of give and take. Please don't take it hard if your
142269
first try is not accepted. It happens to all of us.
143270

144271
8. After approval, one of the senior developers (with commit approval to the
145-
official main repository) will merge your fixes into the master branch.
272+
official main repository) will merge your fixes into the main branch.
273+
274+
Please see the [Code Review](doc/dev/CodeReview.md) document for more
275+
explanaton of the code review process.
146276

147277

148278
Coding Style
@@ -224,9 +354,9 @@ capitalize new words: `class CustomerList;` In general, local variables
224354
should start with lower case. Macros should be `ALL_CAPS`, if used at all.
225355

226356
If your class is extremely similar to, or modeled after, something in the
227-
standard library, Boost, or something else we interoperate with, it's ok to
357+
standard library, or something else we interoperate with, it's ok to
228358
use their naming conventions. For example, very general utility classes and
229-
templates (the kind of thing you would normally find in std or boost) should
359+
templates (the kind of thing you would normally find in std) should
230360
be lower case with underscores separating words, as they would be if they
231361
were standards.
232362

@@ -259,7 +389,7 @@ Namespaces: yes, use them!
259389

260390
#### Third-party libraries
261391

262-
Prefer C++11 `std` rather than Boost or other third party libraries, where
392+
Prefer C++17 `std` rather than other third party libraries, where
263393
both can do the same task.
264394

265395
If you see a third party library already used as a dependency (such as OIIO,

README.md

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -531,13 +531,27 @@ or issue, you may contact the project administrator privately at
531531
[lg@larrygritz.com](lg@larrygritz.com).
532532

533533

534-
Contributing
535-
------------
534+
Contributing and developer documentation
535+
----------------------------------------
536536

537537
OSL welcomes code contributions, and nearly 50 people have done so over the
538538
years. We take code contributions via the usual GitHub pull request (PR)
539-
mechanism. Please see [CONTRIBUTING](CONTRIBUTING.md) for detailed
540-
instructions.
539+
mechanism.
540+
541+
* [CONTRIBUTING](CONTRIBUTING.md) has detailed instructions about the
542+
development process.
543+
* [AI Policy](doc/dev/AI_Policy.md) decribes our policies on AI coding
544+
assistance tools.
545+
* [RELEASING](doc/dev/RELEASING.md) explains our policies and procedures for
546+
making releases. We have a major, possibly-compatibility-breaking, release
547+
annually in September/October, and minor bug fix and safe feature addition
548+
release at the beginning of every month.
549+
* Other developer documentation is in the [doc/dev](doc/dev) directory.
550+
* You may also have luck learning a bit about the organization and
551+
architecture of the project by reading the [DeepWiki Analysis of
552+
OpenShadingLanguage](https://deepwiki.com/AcademySoftwareFoundation/OpenShadingLanguage).
553+
But take it with a grain of salt -- like any LLM-generated summary, there
554+
may be inaccuracies lurking.
541555

542556

543557
Contacts, Links, and References

0 commit comments

Comments
 (0)