Skip to content

Conversation

toscalix
Copy link
Contributor

@toscalix toscalix commented Aug 5, 2025

Description of the properties of the merged list draft. #8

Description of the properties of the merged list draft.
Copy link

@stevenc-stb stevenc-stb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@bobmartin3000 bobmartin3000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@zvr zvr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some things need to be cleared up.

* Asymmetric-key algorithms, commonly known as public-key algorithms, use two related keys (i.e., a key pair) to perform their functions: a public key and a private key. The public key may be known by anyone; the private key should be under the sole control of the entity that “owns” the key pair. Even though the public and private keys of a key pair are related, knowledge of the public key cannot be used to determine the private key.
* Values: "Cryptographic-Hash-Function" , "Symetric-Key-Algorithm" or "Asymmetric-Key-Algorithm"

Note: the subclasses has been added to the cryptoClass property, separated by a "/" character from the class. This specific way to structure the subclasses is WIP.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If "Values" is a fixed set of 3 alternatives, there cannot be subclasses.
The easiest way would be to define something like cryptoSubClass as a separate string, arbitrary for now, to be defined maybe later.

Otherwise you have to explain here the actual structure of the allowed values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ticket reflect the action to mitigate this comment #17

Copy link
Contributor Author

@toscalix toscalix Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to #17 an enhancement of the properties description file has been added to better describe the current state of the list. Addressed in commit f6d9c93

cryptoSubClass property described, with additional notes since this property is not fully consolidated.
@toscalix toscalix requested a review from zvr August 18, 2025 14:17
@toscalix
Copy link
Contributor Author

@zvr given that this is the first PR with reviews, I wonder if you are expecting me to resolve the comments or with the "request for review" is enough and you will be the one resolving the comments.

Adapted the properties description to the new verion of the list:
* Removed the AND operator and adapted the description to the selected yaml compatible way to express several values spdx#23
* Removed the TO operator and adapted the description to the selected yaml compatible way to express a range of values
spdx#26
@zvr zvr merged commit 8df4761 into spdx:main Oct 7, 2025
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.

4 participants