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

Fix NPE when includeStdTypes flag is set, and clean goal is not specified #26

Conversation

spensonshih
Copy link
Contributor

Description

Fixing a NullPointerException when running the goal repeatedly without clean.

Changes Made

The NullPointerException is due to an uninitialized variable for the protocVersion when the plugin detected proto files remain unchanged since last compilation, but the includeStdTypes flag is set.

This pull requests adds the includeStdTypes flag check to prevent the NPE.

Definition of Done

Before submitting this pull request, please ensure that the following criteria have been met:

  • All automated tests have passed successfully.
  • All manual tests have passed successfully.
  • Code has been reviewed by at least one other team member.
  • Code has been properly documented and commented as needed.
  • All new and existing code adheres to our project's coding standards.
  • All dependencies have been added or removed from the project's README or other documentation as needed.
  • Any relevant documentation or help files have been updated to reflect the changes made in this pull request.
  • Any necessary database migrations have been run.
  • Any relevant UI changes have been reviewed and approved by the UI/UX team.

@ee08b397
Copy link

ee08b397 commented May 13, 2024

The change looks good. I found a similar change about "Use of breaks " from the original repo at
os72/protoc-jar-maven-plugin#40.

Found the same change to <includeStdTypes>true</includeStdTypes> in this blog
https://blog.jetbrains.com/idea/2023/05/how-to-work-with-protobuf-maven-projects-in-intellij-idea/

Copy link

@wchen-55e wchen-55e left a comment

Choose a reason for hiding this comment

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

LGTM

@spensonshih spensonshih merged commit a964761 into main May 14, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants