-
Notifications
You must be signed in to change notification settings - Fork 213
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
mixup in case of DISPLAY messages type #1038
Merged
Merged
Conversation
This file contains 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
Cast is wrong, as message can be of multiple type. Fixes #1036
gnodet
requested changes
Jun 19, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's wrong imho.
The type of message identifies how it will be read, see
maven-mvnd/common/src/main/java/org/mvndaemon/mvnd/common/Message.java
Lines 80 to 123 in 2b8076d
case BUILD_REQUEST: | |
return BuildRequest.read(input); | |
case BUILD_STARTED: | |
return BuildStarted.read(input); | |
case BUILD_FINISHED: | |
return BuildFinished.read(input); | |
case MOJO_STARTED: | |
return MojoStartedEvent.read(input); | |
case PROJECT_LOG_MESSAGE: | |
case DISPLAY: | |
return ProjectEvent.read(type, input); | |
case BUILD_EXCEPTION: | |
return BuildException.read(input); | |
case KEEP_ALIVE: | |
return BareMessage.KEEP_ALIVE_SINGLETON; | |
case STOP: | |
return BareMessage.STOP_SINGLETON; | |
case PROMPT: | |
return Prompt.read(input); | |
case PROMPT_RESPONSE: | |
return PromptResponse.read(input); | |
case PROJECT_STARTED: | |
case PROJECT_STOPPED: | |
case BUILD_STATUS: | |
case BUILD_LOG_MESSAGE: | |
return StringMessage.read(type, input); | |
case CANCEL_BUILD: | |
return BareMessage.CANCEL_BUILD_SINGLETON; | |
case TRANSFER_INITIATED: | |
case TRANSFER_STARTED: | |
case TRANSFER_PROGRESSED: | |
case TRANSFER_CORRUPTED: | |
case TRANSFER_SUCCEEDED: | |
case TRANSFER_FAILED: | |
return TransferEvent.read(type, input); | |
case EXECUTION_FAILURE: | |
return ExecutionFailureEvent.read(input); | |
case PRINT_OUT: | |
case PRINT_ERR: | |
return StringMessage.read(type, input); | |
case REQUEST_INPUT: | |
return RequestInput.read(input); | |
case INPUT_DATA: | |
return InputData.read(input); |
So we can't have two message classes for the same message type.
cstamas
changed the title
ClassCastEx in case of some messages
mixup in case of DISPLAY messages type
Jun 20, 2024
gnodet
approved these changes
Jun 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
ProjectEvent message mixup fixed.
Fixes #1036