Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release v0.16.0 changes #1716

Merged

Conversation

reinecke
Copy link
Collaborator

@reinecke reinecke commented Mar 30, 2024

  • drop .dev1 suffix
  • update CONTRIBUTORS.md
  • Added note to the README.md about OpenTimelineIO-Plugins
  • c++ code formatted

This is the release of items from Milestone 18 (Public Beta 16)

@reinecke reinecke added this to the Public Beta 16 milestone Mar 30, 2024
…/github.com/actions/runner-images

* drop .dev1 suffix
* check the README.md for any updates
* update CONTRIBUTORS.md
* c++ code formatted

Signed-off-by: Eric Reinecke <reinecke.eric@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2024

Codecov Report

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).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1716      +/-   ##
==========================================
- Coverage   79.97%   79.95%   -0.02%     
==========================================
  Files         197      197              
  Lines       21867    21879      +12     
  Branches     4338     4342       +4     
==========================================
+ Hits        17488    17494       +6     
- Misses       2249     2252       +3     
- Partials     2130     2133       +3     
Flag Coverage Δ
py-unittests 79.95% <41.02%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/opentimelineio/composition.h 45.16% <100.00%> (ø)
src/opentimelineio/externalReference.h 100.00% <ø> (ø)
src/opentimelineio/generatorReference.h 100.00% <ø> (ø)
src/opentimelineio/imageSequenceReference.h 48.27% <ø> (ø)
src/opentimelineio/item.h 100.00% <100.00%> (ø)
src/opentimelineio/mediaReference.h 100.00% <ø> (ø)
src/opentimelineio/composition.cpp 55.91% <0.00%> (ø)
src/opentimelineio/gap.cpp 88.88% <50.00%> (ø)
src/opentimelineio/deserialization.cpp 35.27% <0.00%> (-0.09%) ⬇️
src/opentimelineio/stackAlgorithm.cpp 29.12% <44.44%> (-0.58%) ⬇️
... and 1 more

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update abe8308...a683399. Read the comment docs.

Copy link
Collaborator

@meshula meshula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whitespace changes aside LGTM

@meshula
Copy link
Collaborator

meshula commented Mar 30, 2024

(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 ;) )

@jminor
Copy link
Collaborator

jminor commented Apr 1, 2024

@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?

@reinecke
Copy link
Collaborator Author

reinecke commented Apr 1, 2024

@jminor sorry - that was a copy/paste error in my change summary. No changes intended to README.

@reinecke
Copy link
Collaborator Author

reinecke commented Apr 1, 2024

@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:

  • Add CI check for C++ lint issues in the CI
  • Add some automation that submits a clang-format PR on any changes to the C++ code

I don't recall if we've discussed either of these before. Do you have strong feelings about either one of these?

@darbyjohnston
Copy link
Contributor

It would also be nice to relax the 80 column limit, I'm not sure some of those formatting changes are an improvement.

@ssteinbach
Copy link
Collaborator

ssteinbach commented Apr 1, 2024

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: 0aa7765

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.

@ssteinbach
Copy link
Collaborator

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.

Signed-off-by: Eric Reinecke <reinecke.eric@gmail.com>
@reinecke
Copy link
Collaborator Author

reinecke commented Apr 1, 2024

@ssteinbach @jminor I made a quick update to the readme noting people should start using OpenTimelineIO-Plugins to get the full compliment of adapters.

README.md Outdated Show resolved Hide resolved
Signed-off-by: Eric Reinecke <reinecke.eric@gmail.com>
@reinecke reinecke merged commit 2b9c3a4 into AcademySoftwareFoundation:main Apr 3, 2024
45 checks passed
@reinecke reinecke deleted the main.release.0.16.0 branch April 3, 2024 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants