Skip to content

Conversation

ahmadsharif1
Copy link
Contributor

No description provided.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 16, 2024
@ahmadsharif1 ahmadsharif1 reopened this Oct 16, 2024
README.md Outdated
Comment on lines 143 to 144
If you build TorchCodec with ENABLE_CUDA=1 or use the CUDA-enabled release of
torchcodec please review
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should wait until we have cuda wheels before we mention the "CUDA-enabled release", otherwise this might be confusing for users?

Suggested change
If you build TorchCodec with ENABLE_CUDA=1 or use the CUDA-enabled release of
torchcodec please review
If you build TorchCodec with ENABLE_CUDA=1, please review

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to get the legal approval for it and not change it last minute. Hence I added a blanket clause. I can add a (when available) to it if you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get legal approval for the full sentence now, land with the suggestion, and re-add the "CUDA-enable release" at the time of release? It doesn't seem much more work than adding "when available" and then having to remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am inclined not to change legally approved sentences.

We can delay merging this PR though

README.md Outdated
However, TorchCodec may be used with code not written by Meta which may be
distributed under different licenses.

For example, if you build TorchCodec with ENABLE_CUDA=1 or use the CUDA-enabled release of torchcodec, please review CUDA's license here: [Nvidia licenses](https://docs.nvidia.com/cuda/eula/index.html).
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider keeping the line length similar to the rest

@ahmadsharif1 ahmadsharif1 merged commit 856fe62 into meta-pytorch:main Nov 14, 2024
13 checks passed
NicolasHug pushed a commit to NicolasHug/torchcodec that referenced this pull request Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants