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.

Sign up for GitHub

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

Jump to bottom

Fixing #315: Guarding against newVersion or property being empty if the properties file is not provided #667

Merged
merged 1 commit into from Sep 5, 2022
Merged

Fixing #315: Guarding against newVersion or property being empty if the properties file is not provided #667

merged 1 commit into from Sep 5, 2022

Conversation

jarmoniuk
Copy link
Contributor

Simple fix + unit test

@jarmoniuk
Fixing mojohaus#315: Guarding against newVersion or property being em…
029d0e8 
…pty if the properties file is not provided
@slawekjaranowski slawekjaranowski linked an issue Sep 5, 2022 that may be closed by this pull request
set-property without newVersion property doesn't give a useful error message #315
Closed
@slawekjaranowski slawekjaranowski merged commit 0db6ce4 into mojohaus:master Sep 5, 2022
@jarmoniuk jarmoniuk deleted the issue-315 branch September 5, 2022 12:25
@Koziol-ku
Copy link

This is a problematic fix, as now we can't unset property, which is already in pom.xml

@slawekjaranowski
Copy link
Member

@Koziol-ku thanks for info.

Please create new issue with example project/pom which can be used to reproduce.

@jarmoniuk
Copy link
Contributor Author

@Koziol-ku could you please elaborate and, if possible, create a ticket with reproduction steps and pom? The idea was to protect against an NPE which occurred if either version or property was empty.

@spyrolabs
Copy link

Using mvn versions:set-property -Dproperty=changelist -DnewVersion= where we have <changelist>-SNAPSHOT</changelist> would have become <changelist></changelist> with version 2.11.0. With version 2.12.0, we have the error explained by @Koziol-ku

@jarmoniuk
Copy link
Contributor Author

Thanks, I'll fix that.

@jarmoniuk jarmoniuk mentioned this pull request Sep 8, 2022
Closed
@jarmoniuk
Copy link
Contributor Author

Created a PR restoring that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

set-property without newVersion property doesn't give a useful error message
4 participants
@jarmoniuk @Koziol-ku @slawekjaranowski @spyrolabs