Add new check for new parameters' default value #5784
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Vincent-lau
changed the title
lindig
approved these changes
Jul 5, 2024
Vincent-lau
force-pushed
the
private/shul2/check-param
branch
from
02d0b9f to
0bc1c71
Compare
snwoods
approved these changes
Jul 9, 2024
minglumlu
reviewed
Jul 11, 2024
| checking parameters with newer releases does not work well as there are existing | ||
| parameters with new releases but no default. *) | ||
| let _ = | ||
| let new_param_has_default obj_name msg_name ps = |
There was a problem hiding this comment.
I think the "new parameter" could be defined as the ones whose release is larger than the release of its message/call.
Initially, the parameters (may be empty) of a message don't have default value defined as they are created with the message together. After that, adding new parameters will make all parameters as versioned_params. To distinguish them, the version of the new parameters must be larger than the message itself.
There was a problem hiding this comment.
Yes, I totally agree that should be the proper way. But there are existing params that violate this way of checking, such as tunnel-create....
lindig
approved these changes
Jul 11, 2024
Vincent-lau
force-pushed
the
private/shul2/check-param
branch
from
7ac6b2c to
0416e2a
Compare
Also move the last function to Xapi_stdext. Signed-off-by: Vincent Liu <[email protected]>
Note we are only checking for default of new parameters in messages that already have default parameters. It lacks checks for new parameters added to messages with no default parameters, but there is no easy way to achieve that because the generator cannot tell what is new and what is not, moreover, there are existing messages with non-default parameters introduced in different releases. Signed-off-by: Vincent Liu <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add new check for new parameters' default value
Note we are only checking for default of new parameters in messages
that already have default parameters.
It lacks checks for new parameters added to messages with no default
parameters, but there is no easy way to achieve that because the generator
cannot tell what is new and what is not, moreover, there are existing
messages with non-default parameters introduced in different releases.