Skip to content

Conversation

@onel
Copy link
Contributor

@onel onel commented Aug 15, 2025

This branch out of onel:reference-docs-20250811_161950/1, so it contains the changes from #1432

@KernelDeimos
Copy link
Contributor

Now that the other PR mentioned has been merged, this can be rebased and the correct changes will be reflected.

I think writing documentation with the perspective that it's for developers adding their own property types would be most beneficial. So you can re-use work that you already did to create maybe a markdown document that can be used as a reference.

Comments above adapt and validate methods don't need to mention that the method is adapting or validating - that's a concern of the implied interface rather than each individual implementation. They can remain in jsdoc format, but it's important to recognize that these particular jsdoc comments won't ever show up in tips from the text editor or IDE since they're almost always called behind other calls from the perspective of a developer using entity storage.

@onel
Copy link
Contributor Author

onel commented Aug 19, 2025

Those are good points. I have some questions.

I think writing documentation with the perspective that it's for developers adding their own property types would be most beneficial. So you can re-use work that you already did to create maybe a markdown document that can be used as a reference.

Do you mean that these docs would better live in a readme file than in the code?
Would the code be better served by types than these docs?

Comments above adapt and validate methods don't need to mention that the method is adapting or validating - that's a concern of the implied interface rather than each individual implementation.

Do you mean that the methods are too simplistic to be documented or that the docs should attached to string, flag and not explain the actual methods?

@KernelDeimos
Copy link
Contributor

KernelDeimos commented Aug 20, 2025

Do you mean that these docs would better live in a readme file than in the code?
Would the code be better served by types than these docs?

These objects all get passed into constructor calls for new PropType objects. The methods on those objects are never called directly except inside PropType, so those particular typings will only ever show up in the editor when someone is working on PropType itself (and only in the constructor if I'm not mistaken). PropType's methods would be a better place for the jsdoc types. The explanation of nuances for individual types could go on a description property of each PropType - that fits the declarative nature of these entries very well.

If your curious, RegistrantService is where these entries become actualy PropType instances.

Do you mean that the methods are too simplistic to be documented or that the docs should attached to string, flag and not explain the actual methods?

These probably should become description properties like I mention above, but what I meant was re-using the verbs "adapts" or "validates" above methods called adapt and validate doesn't add anything new. For example Validates that a string is a valid UUID v4 could instead be true if value is a string and a valid UUIDv4. Unlike other code, these entries are wrapped in a structured format, so repeating the verb is kind of like if you were to describe the meaning of a heading of a table above every cell of that table.

@onel
Copy link
Contributor Author

onel commented Aug 22, 2025

Ohh, I see. That makes perfect sense. What would you recommend?
I think dropping the docs in this file (at least for adapt / validate) and moving the docs up a level to something like RegistrantService?

@KernelDeimos
Copy link
Contributor

My recommendation is described by the previous comment. If an specific detail isn't clear feel free to let me know which it is.

@onel
Copy link
Contributor Author

onel commented Sep 11, 2025

Reread your comments. I think I understnad. Will update this PR

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