Allow customising detector name #86
Conversation
| }; | ||
| const detectorName = core.getInput("detector-name"); | ||
| if (detectorName == "") { |
There was a problem hiding this comment.
Wouldn't this be a !== check, as we would only set this if it is not empty?
There was a problem hiding this comment.
Also if this cannot legitimately be set to an empty/white space string, then we should use a check like: detectorName && detectorName.trim().length > 0 or get the core.getInput() call to do the trimming for us with this option; https://github.com/actions/toolkit/blob/main/packages/core/src/core.ts#L18
There was a problem hiding this comment.
My preference would be that this not be allowed to be empty. I can't say for sure that it would error out, but it just seems like the kind of thing that can only cause problems.
There was a problem hiding this comment.
@peter-murray getInput trims whitespace by default.
| if (detectorName == "") { | ||
| snapshotConfig.detector = { | ||
| name: detectorName, | ||
| url: core.getInput("detector-url"), |
There was a problem hiding this comment.
Can these be empty strings as valid values? I have not looked too deeply into these configuration values. Would we want a default for these if not specified, or just take the potential empty string value from the caller?
There was a problem hiding this comment.
I'm not sure if empty strings would pass validation off the top of my head, but I would encourage setting non-empty default values either way.
There was a problem hiding this comment.
I've made a change so that if detector-name is specified then detector-version and detector-url are required to be specified by the user. There are no sensible defaults we could use here, only dummy ones (e.g. detector-url: https://example.com). Requiring the user to intentionally make that decision feels better to me.
Allow users of the action to customise the detector name. New inputs:
detector-namedetector-versiondetector-urlIf
detector-nameis not specified, then information extracted frompackage.jsonis used, as before (no changes).