TSC Meeting Notes 2020-09-10
Attending:
- Cary Phillips
- Christina Tempelaar-Lietz
- Joseph Goldstone
- Kimball Thurston
- Nick Porcino
- Peter Hillman
Discussion:
Need help with Issue #821, Damaged OpenEXR files can cause Undefined-shift operators in B44 uncompress. The expected behavior is not clear. Cary will contact Florian, who appears to be the original author.
Discussion of exception handling:
Imath PR #47 introduces Frustum::projectPointToScreen_noexcept(), but that's inconsistent with Vec3::normalize() and Vec3::normalizeExc().
Kimball: If we were designing it today, we'd write separate classes, one that handles/throws exceptions and one that doesn't.
Peter: but sometimes the throw/don't-throw decision is in the use (tight loop), not in the definition of the data itself.
We assume the most critical use case is code that must be compiled with a compiler that doesn't support exceptions (games code).
The consensus is: Continue the paradigm of the Vec classes: the ordinary function name does not throw; a parallel function with the suffice "Exc" does throw. We also wrap the throwing code in #if's so that it's possible to omit it at compile time.
More untangling is necessary. The integer specializations of Vec::normalize() throw if the vector is not along a principal axis. And Matrix22::inverse(bool) takes an argument. These should all be made consistent. And noexcept added.
Discussion of branch names. It came up again at yesterday's TAC meeting. Larry seems to favor the emerging GitHub conventions. Kimball still prefers dev/release.
Discussion of the default branch: consensus is it should be "release". If someone clones the repo, they should get the release branch. That way, they build off of what is closest to the released code, although it may not be an actual release, because PR's may have been merged into the branch since the latest release. But that's still reasonable. Soon, our dev branch is going to contain major changes, so it's good that you have to be intentional about working with it.
The name "release" sounds like it's an actual release, but it's not. Would it be better called "releases", since it's the branch that has the record of all past releases?
The typical download/install process should download the tarball or pull from a specific release tag (v2.5.3), but we can't assume all inexperienced users do that. Better to make the typical behavior as understandable as possible.
When someone clones the repo, fixes a bug, and submits a PR, that PR is then likely to be against the release branch, not dev/main. That's OK, even good. During the review of the PR, we (maintainers) should discuss whether the change should 1) go into dev instead, 2) get merged into dev as well, or 2) go into release only. Cary: better to make this decision and do the merge as we go, PR by PR, rather than letting them accumulate and then having to do a bunch of merging and cherry picking prior to a release.
In the two-branch workflow, we will always pay attention to what branch a PR goes against. Not an issue now, but see other advantages to having two releases, so this seems reasonable.