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

API 24 and tasks docs #176

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open

API 24 and tasks docs #176

wants to merge 33 commits into from

Conversation

crandmck
Copy link
Collaborator

@crandmck crandmck commented Nov 14, 2024

Changes in this pull request

Clarify and document new API and add manifest tasks.

Developers should review the languages that they work on. Do we need further reviewers?

For information on where all the files are, see Notes at the bottom of this description.

Rust

@gpeacock and @andrewhalle to review

C++

@gpeacock to review

Python

@tmathern to review

JavaScript

@emensch to review

Node.js

@cdmurph32 to review

Notes

To make reviewing easier for developers, I moved all the code into separate files in the docs/tasks/includes directory:

  • JavaScript code is in files named _javascript-*.md
  • C++ code is in files named _cpp-*.md
  • Node.js code is in files named _node-*.md
  • Python code is in files named _python-*.md
  • Rust code is in files named _rust-*.md

Each language has four files, one for each task:

  • Preliminary setup: *-setup.md
  • Reading manifest data: *-read.md
  • Getting resources from a manifest: *-get-resources.md
  • Attaching a manifest to an asset and signing it: *-build.md

Add manifest tasks to sidebar nav.
@crandmck crandmck marked this pull request as draft November 14, 2024 20:10
Copy link

github-actions bot commented Nov 14, 2024

@github-actions github-actions bot temporarily deployed to pull request November 14, 2024 20:12 Inactive
@github-actions github-actions bot temporarily deployed to pull request November 15, 2024 00:55 Inactive
@crandmck crandmck marked this pull request as ready for review December 3, 2024 22:51
@crandmck crandmck marked this pull request as draft December 3, 2024 22:51
@github-actions github-actions bot temporarily deployed to pull request January 10, 2025 19:46 Inactive
@github-actions github-actions bot temporarily deployed to pull request January 15, 2025 00:00 Inactive
@github-actions github-actions bot temporarily deployed to pull request January 15, 2025 21:33 Inactive
@github-actions github-actions bot temporarily deployed to pull request January 15, 2025 22:25 Inactive
@crandmck crandmck mentioned this pull request Jan 29, 2025
@github-actions github-actions bot temporarily deployed to pull request February 12, 2025 22:49 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 14, 2025 22:45 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 14, 2025 23:02 Inactive
@tmathern
Copy link

https://deploy-preview-176--cai-open-source.netlify.app/docs/tasks/build/?lang=python : FROM SIGNING is all lower case in the doc, but I think that's not right.

Otherwise, LGTM.

@cdmurph32
Copy link

I'm not sure what wasm support means with the check boxes. The almost the entire SDK supports wasm. The only major gap is openSSL signing and verification for ES512. Does that column mean JavaScript?

@crandmck
Copy link
Collaborator Author

crandmck commented Feb 20, 2025

I'm not sure what wasm support means with the check boxes. The almost the entire SDK supports wasm. The only major gap is openSSL signing and verification for ES512. Does that column mean JavaScript?

@cdmurph32 I'm not sure what you're referring to, perhaps the Rust release notes...? That's not part of this PR.

I'm asking you to review these pages:

In particular, "Getting resources" needs something!

The relevant files in the PR are:

  • Setup: docs/tasks/includes/_node-setup.md
  • Reading manifest data: docs/tasks/includes/_node-read.md
  • Getting resources from a manifest: docs/tasks/includes/_node-get-resources.md
  • Attaching a manifest to an asset and signing it: docs/tasks/includes/_node-build.md

@crandmck
Copy link
Collaborator Author

https://deploy-preview-176--cai-open-source.netlify.app/docs/tasks/build/?lang=python : FROM SIGNING is all lower case in the doc, but I think that's not right.

Oops, that whole section was essentially a duplicate of the previous code block, that I forgot to remove.... I deleted it, so it should look better now.

@crandmck crandmck requested a review from tmathern February 21, 2025 00:04
@github-actions github-actions bot temporarily deployed to pull request February 21, 2025 00:04 Inactive
Copy link
Contributor

Choose a reason for hiding this comment

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

This setup works, but I think it would be useful to demonstrate how to include the library when installed locally (e.g. via npm). For the library itself, that just looks like:

import { createC2pa } from 'c2pa':

But it gets a little tricker for the wasmSrc and workerSrc, which need to be made available as static assets that can be fetched at runtime. The best way to do that will vary depending on the build system. For example, vite has an "explicit URL imports" feature that we use in our minimal-ts-vite example.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@github-actions github-actions bot temporarily deployed to pull request February 21, 2025 20:37 Inactive
@tmathern
Copy link

For nodejs:

https://deploy-preview-176--cai-open-source.netlify.app/docs/tasks/setup/?lang=node

  • import { readFile } from 'node:fs/promises'; looks unusual, but if it's in our code this way and works I would leave it (more usual is ... from fs.promises)

https://deploy-preview-176--cai-open-source.netlify.app/docs/tasks/read/?lang=node

  • You may want to add that the path param needs to be a valid path (since we don't revalidate in the example), and mimeType a valid and supported mimetype (again because in the example we don't revalidate and/or check for undefined/null)

https://deploy-preview-176--cai-open-source.netlify.app/docs/tasks/build/?lang=node

  • FROM SIGNING is all uppercase somewhere in that page.
  • If you have an asset file's data loaded into memory, you can sign the the asset using the loaded stream (buffer). Streams are more native for node (and js), so you can be a little shorter here in the explanation: If you have an asset file's data loaded into a stream, you can use it to sign the asset

https://deploy-preview-176--cai-open-source.netlify.app/docs/tasks/get-resources/?lang=node

  • I didn't find examples/test either. But if we want to showcase it, I think we can get inspiration from the js code (the first few lines).

cc @cdmurph32

@github-actions github-actions bot temporarily deployed to pull request March 10, 2025 21:09 Inactive
@github-actions github-actions bot temporarily deployed to pull request March 10, 2025 21:15 Inactive
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.

6 participants