Skip to content
New issue

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

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

Already on GitHub? # to your account

Fixes potential NullPointerException in ExpressionNode::toString #269

Merged
merged 1 commit into from
Sep 11, 2018

Conversation

spilot
Copy link
Contributor

@spilot spilot commented Sep 11, 2018

The NPE is thrown when SourceSection == null

@smarr smarr changed the base branch from release to dev September 11, 2018 09:31
@smarr smarr added the bug Fixes an issue, incorrect implementation label Sep 11, 2018
@smarr smarr added this to the v0.7.0 milestone Sep 11, 2018
Copy link
Owner

@smarr smarr left a comment

Choose a reason for hiding this comment

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

Good catch. Thank you!

@smarr
Copy link
Owner

smarr commented Sep 11, 2018

Just a bit on process:

I changed the base for the PR to the dev branch. Usually, we keep release unchanged, even for bug fixes. Our project isn't large enough to be able to maintain more than one branch unfortunately.

And, then a tip for using branches/github: if you would push the change to a separate branch, it is easier to iterate on more complex changes, and then simply push to that branch. After it is merged, it can also be simply deleted again. Another benefit is that your own main development branch doesn't depend on the progress of the PR.

As for this change, it looks all good. Tests simply failed, because the release branch currently fails building. Think that's an issue with missing libraries, things that moved on the internet. Checkstyle for instance.

@smarr smarr merged commit f2a5855 into smarr:dev Sep 11, 2018
@spilot
Copy link
Contributor Author

spilot commented Sep 11, 2018

Got it, thank you! :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Fixes an issue, incorrect implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants