Docstrings for package json helpers #8373
Conversation
pr7prashant
commented
Sep 3, 2022
pr7prashant
commented
- Added docstrings for package json helpers
|
| * Checks if the object has all the dependency types and the dependency types are valid. | ||
| * | ||
| * @type {object} | ||
| */ |
There was a problem hiding this comment.
Because Joi schemas are so commonly used, we've actually set up jsdoc so that we can use Joi as a "type" for schemas
Lines 170 to 171 in daca767
e.g:
shields/core/base-service/base-json.js
Line 29 in daca767
I guess we've probably merged a few of these PRs describing the schemas as object but it lets use @type {Joi} for Joi schemas moving forwards.
There was a problem hiding this comment.
Noted. Joi type does make more sense here. Will remember this moving forward.
services/package-json-helpers.js
Outdated
| * @param {object} attrs.optionalDependencies - Map of optional dependencies | ||
| * @throws {string} - Error message if unknown dependency type provided | ||
| * @throws {InvalidParameter} - Error if wanted dependency is not present | ||
| * @returns {object} Object containing wanted dependency version |
There was a problem hiding this comment.
This function does currently return an object, but it doesn't really need to. I expect this is an artefact of some past refactoring. A nice cleanup we could do here would be to update this so it just returns a string and update the handful of places it is called accordingly. Also, the return value (range) might be an exact version, but it might also be a semver range i.e: this value could be something like "~2.1.6" or ">=3.0.0,<4.0.0" rather than a precise version number so lets explain that in the docstring.
There was a problem hiding this comment.
Sure, will update this description. I'll raise a new PR for the cleanup to keep it simple.
