Docs: Line wrap long lines of text for a display

[ThomasWilshaw]

trafficstars

Link the Issue(s) this Pull Request is related to.

Fix #1139 Summarize your change.

The line length of some of the comments in the code blocks was extreamly long making reading it a pain. This PR manually line wraps that to 80 characters. I donlt know if there is a specific convention for this.

[ssteinbach]

This documentation is actually autogenerated, by extracting from pydoc directly from the classes. So the line breaking would have to happen in this script:

https://github.com/PixarAnimationStudios/OpenTimelineIO/blob/27ed98663fb4e20b9e187f85b34f5a58f9bd60d9/src/py-opentimelineio/opentimelineio/console/autogen_serialized_datamodel.py#L319

[ThomasWilshaw]

Might it be better to wrap the lines here rather than trying to do it automatically and risking wrapping very long commands that should probably stay on a single line?

[ssteinbach]

Thats an option. There are also tools in python for wrapping text: https://docs.python.org/3/library/textwrap.html

@meshula I'm a little surprised our clang format pass didn't wrap these lines, actually.

[JeanChristopheMorinPerso]

@ssteinbach those string are prefixed with R, which I guess it's kind of a raw string. It would be weird if clang formatted these raw string no?

[ThomasWilshaw]

How does that look? It's been a while since Ive written any Python code so there may be a nicer/better way to do it.

[ssteinbach]

The unit tests check to see if the autogenerated docs have been updated -- you'll need to check in to your branch the updated docs. You can generate them by using the make doc-model-update build target.

[ThomasWilshaw]

Done, but for some reason there has been a change in the text that's nothing to do with the Python code

[meshula]

For certain, we would never want clang-format to interfere with an R"string", as that would interfere with the very purpose of the raw strings.

[ThomasWilshaw]

I've just updated this. I had an older version of OTIO in my Python setup that was messing things up

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: ThomasWilshaw (fb65bf4098847f01ecd4449af6be44bb073f1cbe, 09e19922c8b087b1f55edfce8ce12c6643e34763, fa91d707b2dd682d61fab5ff34f7621eb467b0cf, 7124ccb5480691866bda67ea52117cbb096a29f8)
• :white_check_mark: login: meshula / name: Nick Porcino (d564a165b45358eda98da4c0123129821067b016)

[meshula]

@ThomasWilshaw Sorry to have missed your request for a review, I just spotted this. Now, there is the CLA to take care of. If you're able to go through the CLA business, we can get this PR moving again. Apologies again.

[ThomasWilshaw]

No problem, sorry I've dropped off the radar for a while, I've been very busy of late. That's the CLA all done, please let me know if there are any other issues.

[meshula]

Looks like CI is stumbling as soon as it hits the py_build_test. My python-fu is too weak ~ any thoughts on what's tripping it up?

[meshula]

@ThomasWilshaw I corrected the PEP8 issues that came up, but it looks like there needs to be a corresponding update to the test suite to accommodate the reformatting?

[JeanChristopheMorinPerso]

I believe make doc-model-update should do the trick.

[jminor]

It looks like this PR is almost ready. It just needs make doc-model-update and DCO signoff.

[ThomasWilshaw]

Apologies for the massive massive delay on completing this but I think it is all good to go now aside form the DCO stuff. Would someone be able to advise on how I'm supposed to sort that?

[meshula]

The DCO requirement would have needed you to do all your commits with the -s argument. That would be tedious to retrofit. Instead, when you are ready to merge, we'll do a squash merge after forcing the DCO to pass. If you could do one last commit, with -s, it'll be easy enough for us to force that DCO signoff to apply to whole thing.

[JeanChristopheMorinPerso]

BTW, you can see the command line to use at https://github.com/AcademySoftwareFoundation/OpenTimelineIO/runs/10531037992.

[jminor]

It is pretty easy to retroactively sign-off all of the commits like this:

% git status
...check that you're on the right branch ("line_wrap" in your case)
% git rebase main --signoff
...check that the history looks fine (code is unchanged by this command)
% git push --force

The signoff step is intended to make sure you've checked that all of the commits are what you've intended to contribute, and nothing else accidentally got checked in.

More details here if you're curious: https://github.com/AcademySoftwareFoundation/OpenTimelineIO/pull/1140/checks

[jminor]

Oops, it looks like we're all helping to answer at the same time. I didn't see @meshula 's or @JeanChristopheMorinPerso 's advice before posting. Luckily these are all good, mostly overlapping, options. If the commands above don't work for you, let us know and we can do the step that @meshula suggested on your behalf.

[meshula]

@jminor TIL a non-tedious way to fix the sign-off requirement ;)

[ThomasWilshaw]

Thanks for all the suggestions and help. I wasn't able to rebase without some complicated merge conflicts so I've amended the last commit to be signed off as per @meshula's comment.

I assume going forward I should signoff any and all commits to this repo?

[meshula]

I should signoff any and all commits to this repo

Yes, I have to remind myself as well, to add a -S argument to commits.