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

implemented typedoc documentation #907

Merged
merged 10 commits into from
Sep 10, 2024
Merged

Conversation

msantic
Copy link
Contributor

@msantic msantic commented Sep 1, 2024

Addresses #906

Copy link

google-cla bot commented Sep 1, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link

codecov bot commented Sep 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.40%. Comparing base (d437097) to head (da5ea2e).
Report is 90 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #907      +/-   ##
==========================================
- Coverage   91.84%   88.40%   -3.44%     
==========================================
  Files          37       62      +25     
  Lines        4976     8660    +3684     
  Branches        0     1059    +1059     
==========================================
+ Hits         4570     7656    +3086     
- Misses        406     1004     +598     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@elalish elalish left a comment

Choose a reason for hiding this comment

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

Any trouble with signing the CLA? It should be automatic as an individual.

bindings/wasm/package.json Show resolved Hide resolved
@elalish
Copy link
Owner

elalish commented Sep 2, 2024

It'd be great if you can take a stab at it - I think you'll probably just have to modify deploy.yml to build that site and put it in another directory under ./public. I'll take care of trouble-shooting it when we merge, since there will inevitably be some path error with the website the first time it deploys.

I realized we didn't copy our docs for Mesh into our TS declarations, which is why they're missing in your website. Would you mind copying them over and checking the page gets generated correctly? While you're at it, it would be great to have a second set of eyes on the rest of those doc comments in case they've gotten out of sync with the C++.

@msantic
Copy link
Contributor Author

msantic commented Sep 3, 2024

It'd be great if you can take a stab at it - I think you'll probably just have to modify deploy.yml to build that site and put it in another directory under ./public. I'll take care of trouble-shooting it when we merge, since there will inevitably be some path error with the website the first time it deploys.

I'm doing GitHub Pages&Workflow for the first time so hopefully I've done it ok. You can check it out...

I realized we didn't copy our docs for Mesh into our TS declarations, which is why they're missing in your website. Would you mind copying them over and checking the page gets generated correctly? While you're at it, it would be great to have a second set of eyes on the rest of those doc comments in case they've gotten out of sync with the C++.
Sure, no problem. Still learning the codebase so could you please point me where C++ Mesh definition is (the one from manifold-encapsulated-types.d.ts)? There are:

I see also that methods: getCircularSegments, resetToCircularDefaults, setCircularSegments and setMinCircularEdgeLength doesn't have comments. I will fixed them as well.

@elalish
Copy link
Owner

elalish commented Sep 3, 2024

Thanks! The TS Mesh is MeshGL - the old C++ Mesh is deprecated.

@msantic
Copy link
Contributor Author

msantic commented Sep 3, 2024

@elalish I went through the entire doc comments and already fixed a few. Here’s what is still missing:

  • resetToCircularDefaults - ResetToDefaults missing doc in C++, public.h:579
  • Mesh methods:
  • merge(): boolean;
  • verts(tri: number): SealedUint32Array<3>;
  • position(vert: number): SealedFloat32Array<3>;
  • extras(vert: number): Float32Array;
  • tangent(halfedge: number): SealedFloat32Array<4>;
  • transform(run: number): Mat4;

@elalish
Copy link
Owner

elalish commented Sep 3, 2024

Great, thank you! Are those function definitions missing entirely? Feel free to add them.

@msantic
Copy link
Contributor Author

msantic commented Sep 3, 2024

@elalish Yes, the definitions are entirely missing. I could try to add doc comments, but I think it would be better for someone more familiar with the internal logic to do it.

@elalish elalish merged commit 51ee91e into elalish:master Sep 10, 2024
22 checks passed
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.

2 participants