feat: Support properties on license#1383
Conversation
jkowalleck
left a comment
There was a problem hiding this comment.
Thank you for working on this feature.
See my remarks below
src/spec/_protocol.ts
Outdated
| supportsMetadataProperties: boolean | ||
| supportsExternalReferenceHashes: boolean | ||
| supportsLicenseAcknowledgement: boolean | ||
| supportsLicenseProperties: boolean |
There was a problem hiding this comment.
please, dont add a new property supportsLicenseProperties for this.
instead, alter the existing function supportsProperties() accordingly. something like
supportsProperties (model: any): boolean {
switch (true) {
case model instanceof NamedLicense || model instanceof SpdxLicense:
return this.#supportsLicenseProperties
default:
return this.#supportsProperties
}
}the idea is: we dont have a public spec.supportsSomethingProperties for each and every thing. instead, we have the method spec.supportsProperties(someDataModel)
src/spec/_protocol.ts
Outdated
| supportsMetadataProperties: boolean, | ||
| supportsExternalReferenceHashes: boolean, | ||
| supportsLicenseAcknowledgement: boolean, | ||
| supportsLicenseProperties: boolean, |
There was a problem hiding this comment.
move the new argument at the bottom,
or move it behind the supportsProperties
src/serialize/json/normalize.ts
Outdated
| url: JsonSchema.isIriReference(url) | ||
| ? url | ||
| : undefined, | ||
| properties: spec.supportsProperties(data) && spec.supportsLicenseProperties && data.properties.size > 0 |
There was a problem hiding this comment.
| properties: spec.supportsProperties(data) && spec.supportsLicenseProperties && data.properties.size > 0 | |
| properties: data.properties.size > 0 && spec.supportsProperties(data) |
src/serialize/xml/normalize.ts
Outdated
| const url = escapeUri(data.url?.toString()) | ||
| const spec = this._factory.spec | ||
| const url = escapeUri(data.url?.toString()) | ||
| const properties: SimpleXml.Element | undefined = spec.supportsProperties(data) && spec.supportsLicenseProperties && data.properties.size > 0 |
There was a problem hiding this comment.
| const properties: SimpleXml.Element | undefined = spec.supportsProperties(data) && spec.supportsLicenseProperties && data.properties.size > 0 | |
| const properties: SimpleXml.Element | undefined = data.properties.size > 0 && spec.supportsProperties(data) |
src/serialize/xml/normalize.ts
Outdated
| const url = escapeUri(data.url?.toString()) | ||
| const spec = this._factory.spec | ||
| const url = escapeUri(data.url?.toString()) | ||
| const properties: SimpleXml.Element | undefined = spec.supportsProperties(data) && spec.supportsLicenseProperties && data.properties.size > 0 |
There was a problem hiding this comment.
| const properties: SimpleXml.Element | undefined = spec.supportsProperties(data) && spec.supportsLicenseProperties && data.properties.size > 0 | |
| const properties: SimpleXml.Element | undefined = data.properties.size > 0 && spec.supportsProperties(data) |
Signed-off-by: Peter Schuster <p.schuster@pilz.de>
4cef7ee to
2bb7604
Compare
|
Thanks for the review and feedback. The new version of the diff should address all comments and requests for changes. |
Description
Adds support for "properties" on named and spdx licenses available with since schema version 1.5
AI Tool Disclosure
Affirmation