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

Added checking is property not empty string #516

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

idziakjakub
Copy link

This change fixes null return when property is empty string.
When property was empty default value has never been returned.

@tmotyl
Copy link
Collaborator

tmotyl commented Mar 1, 2017

This patch also fixed division by zero issue.

@tmotyl
Copy link
Collaborator

tmotyl commented Jun 12, 2017

@dweeves any feedback on this?

@dweeves
Copy link
Owner

dweeves commented Jun 13, 2017

hi, i was hesitating to merge because of possible impact for purposedly empty values in config files for plugins (which inherit from properties).
i don't think there are many (or even if they are any in fact) , but if there are, this will break the plugin testing explicitely for empty value on one of its settings.
So , maybe a better fix would be in magmi_config parsing rather than ignoring empty values in properties .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants