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

Remove unnecessary warning #28

Merged
merged 2 commits into from
Dec 5, 2017
Merged

Remove unnecessary warning #28

merged 2 commits into from
Dec 5, 2017

Conversation

niezbop
Copy link
Member

@niezbop niezbop commented Dec 4, 2017

No description provided.

@niezbop niezbop self-assigned this Dec 4, 2017
@niezbop niezbop requested a review from lacostej December 4, 2017 15:52
lacostej
lacostej reviewed Dec 4, 2017
View reviewed changes
Copy link
Member

@lacostej lacostej left a comment

Choose a reason for hiding this comment

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

An alternative is to use a static or if we are concerned about multi-threading a [ThreadStatic] value or ThreadLocal instance to not have to propagate the verbose bit and change the method signatures.

@niezbop
Copy link
Member Author

niezbop commented Dec 4, 2017

This is not so much an issue with verbose. ParseIncomplete warns the user if it find identifiers (letters and such) in the version it is trying to parse. Nonetheless, sometimes, we want to be able to get rid of the warning, when parsing Unity versions for instance.

Maybe I should rename the verbose argument I added to something like warnFormat so it is clearer what the intent is?

@lacostej
Copy link
Member

lacostej commented Dec 4, 2017

"warnFormat" better than verbose.

lacostej
lacostej approved these changes Dec 5, 2017
View reviewed changes
Copy link
Member

@lacostej lacostej left a comment

Choose a reason for hiding this comment

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

Great! Thanks

@niezbop niezbop merged commit 69e1c5a into DragonBox:master Dec 5, 2017
@niezbop niezbop deleted the fix/not_founded_warning branch December 5, 2017 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lacostej lacostej lacostej approved these changes

Assignees

@niezbop niezbop

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@niezbop @lacostej