Closed reinecke closed 3 months ago
Attention: Patch coverage is 41.02564%
with 23 lines
in your changes are missing coverage. Please review.
Project coverage is 79.95%. Comparing base (
abe8308
) to head (a683399
).
(I'm not objecting to landing the ws changes, just noting that I checked to see if anything changed within the ws and didn't spot such, a separate ws PR would've required less brain juice ;) )
@reinecke you mentioned "check the README.md for any updates" in the description, but I don't see any README changes in this PR. Did I misunderstand something?
@jminor sorry - that was a copy/paste error in my change summary. No changes intended to README.
@meshula We currently have running clang-format as part of our release steps - I agree that those changes are a bit nerve-racking for this step in the process.
Seems like maybe we should do one of the following:
I don't recall if we've discussed either of these before. Do you have strong feelings about either one of these?
It would also be nice to relax the 80 column limit, I'm not sure some of those formatting changes are an improvement.
It might make more sense to have a deliberate command that executes the C++ format. The reason we landed on this pattern was that we can be permissive as people work and conform the style together in one shot to make sure that releases have consistent formatting. Typically, I would run the C++ format as a separate command and poke through the code, eg: https://github.com/AcademySoftwareFoundation/OpenTimelineIO/commit/0aa7765b6f620af9e456bd93509eff473c527e3a
that would let the CI run and folks look at stuff before landing it.
That said, since I was largely doing the releases on my own, katamari-ing everything together was less overhead for me.
The changes themselves look good to me.
oh yeah - and the checklist suggests reading through the readme to see if anything changed. We might want to add something to the readme for this release noting that its the last release with all the adapters together? I'm not sure off the top of my head of anything else that should be added there.
@ssteinbach @jminor I made a quick update to the readme noting people should start using OpenTimelineIO-Plugins
to get the full compliment of adapters.
This is the release of items from Milestone 18 (Public Beta 16)