Hi@GregStanton ! Quick update here: I've added minimal documentation and fixes to example code here:#7749 Please feel free to revise the text in any way that makes sense - though I believe everything is in there, some points could be much more clear. Thank so much for your work on this!

Sounds good@ksen0! Thank you so much for putting those docs together. Since all tests are passing in this code reorg, would it make sense for me to copy over your docs, and then make any revisions to them here? I'll hold off on that for now, in case it's easier for you or@davepagurek to review the reorg by itself, without extra commits.

Future beginner-friendly issue to improve test coverage and structure

Maybe after the docs are updated, we could make an issue with the "Good First Issue" and "Help Wanted" labels, so that we can give beginners an opportunity to improve the coverage and structure of tests for custom shapes? (Maybe it could work as a good first issue if we provide some support in case reorganization breaks things.)I'll record a few notes for that potential issue below, before I forget what the problems are. I wouldn't mind if you skip them for now 🙂

Coverage

One thing I noticed when updating imports and tests is that we seem to be lacking full test coverage intest/unit/core/vertex.js.

The 1.x version only has tests forbeginShape()/endShape(),vertex(),quadraticVertex(),bezierVertex(), andcurveVertex(). But there are several other functions in the 1.x Vertex module:beginContour()/endContour() andnormal(). Based on a search of 1.x, it looks likenormal() is the only one of these with a unit test, but for some reason it's in its own file intest/unit/webgl/normal.js. The 2.0 version is also missing tests for some functions (at least in test/unit/core/vertex.js), and it doesn't do parameter checks like 1.x does (although perhaps there was a reason for that?).

Structure

There's also some unnecessary friction for contributors, since the file structure of the unit-test directory doesn't reflect the new file structure for 2.0. For example, undersrc,shape was a subdirectory ofcore in 1.x but isn't in 2.0; however, the unit test folder in 2.0 still hasshape undercore. (The 2.0 file structure makes the organization ofsrc more consistent with the organization of the main reference page on the website, which is helpful for new contributors who already use the website reference. So I think we'd want to change the file structure oftest/unit rather thansrc).

@GregStanton copying the updated docs to your branch sounds like the right next step! if you have the time, feel free to start that process, and I'm also happy to help after the Easter weekend too! I think I should be OK reviewing regardless of the extra commits.

Hi both!@GregStanton yes that seems like a great way forward. Please don't feel like you need to keep any of the text I added, you're welcome to organize it as best makes sense. Merging the dev-2.0 branch into this might be a challenge, I see there's some merge conflicts, so please feel free ping if you need help with that, either on Discord or viapublic office hours!

Test coverage

Yes that would be great! Your notes on coverage make total sense. I support reorganization to improve how navigable the codebase it, but for bulk changes (including on test suite), it would be better to keep reorg separate from substantive changes.

Additionally, I think it might be good to include recent bugs that were patched that touch the Shape API (e.g.,#7750) as potential inspiration of what a test might cover.

Finally, a rule of thumb is that if a function was moved from one file or another, just make sure there's 1 more meaningful test added. Really important these aren't AI-generated, because a good test suite covered both very typical and sort of odd cases, and AI might generate OK typical cases, but is unlikely to generate good strange cases.

If I understand all this right, the test coverage issue can already be open and available for work - it's totally independent of this documentation work?

Thanks@davepagurek! I'm going to get to work on this as soon as I can.

@ksen0: Thanks! I'll let you know if I need help with conflicts. Also, great points about using#7750 as a model, and about the rule of thumb for moving functions. I can mention that in the test issue if I'm the one to open it, along with the advice regarding AI.

Makes sense about doing the reorg and the new tests separately. Seems like a good case for GitHub's new sub-issue feature. There could be one issue for the shape tests, with a sub-issue for the reorg and another sub-issue for the new tests. That way, the work can be made separate but can still be coordinated easily.

If I understand all this right, the test coverage issue can already be open and available for work - it's totally independent of this documentation work?

It's independent technically, but I think there are blockers. Basically, some of the shape docs don't fully describe the new behaviors, and it looks like some of the descriptions are incorrect. I'm thinking that anyone writing tests may want access to complete, correct docs. So I figure the current PR should be completed and merged first.