-
Notifications
You must be signed in to change notification settings - Fork 179
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
chore/jsdoc-comments-for-createSite #201
chore/jsdoc-comments-for-createSite #201
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good description to me.
We will have to think how (, how much, and where) to use JSDoc in the project.
E.g. if we use the type multiple times, should we use @typedef
or so? 🤷
(I'm not that proficient with JSDoc, so I searched how to do reusable types...)
Just some open questions...
(Results to those should also be added to the CONTRIBUTING.md
)
@nobkd it's a tricky question, because normally commenting is necessary for a published API. To be honest I was investigating a tricky refactor to inject settings from site.yaml, and was quite impacted with the lack of typings so any option that helps with this will be greatly appreciated. (so we might pull the discussion forward sooner than later.) Just my thoughts. I'll share the tricky branch a bit later tomorrow, since I think it's a good example of a small change in behavior impacting lots of files, and thus relevant to the typings/docs discussion. |
If we take this route, I think we should use jsdoc comments consistently, and everywhere. Or what do you think? I'm personally more into skipping jsdocs and favor unit tests. |
@tipiirai I'm always in favor of tests that tell you how a system is supposed to behave. Especially if we want to support ongoing refactorings. I suggest we close this PR; we can always decide later we now want to start using jsdoc et al once the API has stabilised? |
@tipiirai perhaps just convert this to a dev comment. |
Cool. Closing this one. Thanks @goblinfactory ! |
Add js docs comments on createSite Clarifying the code responsibility.