Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify that all command arguments need to be present #137

Merged
merged 2 commits into from
Jan 22, 2024

Conversation

otterdahl
Copy link
Contributor

No description provided.

If not all command arguments are present, it will result
in a MessageNotAck.
@otterdahl otterdahl changed the base branch from master to 3.2.2 November 21, 2023 12:10
This was referenced Nov 21, 2023
@otterdahl otterdahl added this to the 3.2.2 milestone Nov 21, 2023
@marcgarba
Copy link

Instead of a restrictive "All arguments needs to included in a command, otherwise it results a serious
error resulting in MessageNotAck.", it could be written : "All required arguments for a command needs to be included..."
It would allow the possibility to have some optional arguments in a command.

@otterdahl
Copy link
Contributor Author

otterdahl commented Jan 19, 2024

Instead of a restrictive "All arguments needs to included in a command, otherwise it results a serious error resulting in MessageNotAck.", it could be written : "All required arguments for a command needs to be included..." It would allow the possibility to have some optional arguments in a command.

Good idea. However, right now we're assuming that all arguments are present. Otherwise we might end up with an undefined behaviour of how the TLC is expected to act. But I can certainly agree about there being arguments in some command that can be considered optional, but they need to be defined in the SXL.

We've opened up a separate issue about optional arguments in commands. #148

@otterdahl otterdahl merged commit 8057c35 into 3.2.2 Jan 22, 2024
@otterdahl otterdahl deleted the command_all_arguments branch January 22, 2024 13:37
@otterdahl
Copy link
Contributor Author

I'm merging this and closing this issue.

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.

None yet

2 participants