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

Revert old behaviour for Slim String converter #687

Merged
merged 1 commit into from Mar 31, 2015

Conversation

Projects
None yet
3 participants
@amolenaar
Collaborator

amolenaar commented Mar 21, 2015

String converter used to pass strings as is. The 20150226 version converts empty Strings to null values.

This PR reverts the old behaviour.

String converter should return empty string if empty string is provid…
…ed. This is different from any other converter. Fixes #686.
@mgaertne

This comment has been minimized.

Collaborator

mgaertne commented Mar 21, 2015

What about a feature flag to smooth out the transition here?

Best Markus

Dipl.-Inform. Markus Gärtner
Author of ATDD by Example - A Practical Guide to Acceptance
Test-Driven Development

http://www.shino.de/blog
http://www.mgaertne.de
http://www.it-agile.de
Twitter: @mgaertne

On 21.03.2015, at 17:44, Arjan Molenaar notifications@github.com wrote:

String converter used to pass strings as is. The 20150226 version converts empty Strings to null values.

This PR reverts the old behaviour.

You can view, comment on, or merge this pull request online at:

#687

Commit Summary

String converter should return empty string if empty string is provided. This is different from any other converter. Fixes #686.
File Changes

M src/fitnesse/slim/converters/StringConverter.java (2)
M test/fitnesse/slim/ConverterSupportTest.java (2)
M test/fitnesse/slim/ListExecutorTestBase.java (4)
M test/fitnesse/slim/converters/StringConverterTest.java (25)
Patch Links:

https://github.com/unclebob/fitnesse/pull/687.patch
https://github.com/unclebob/fitnesse/pull/687.diff

Reply to this email directly or view it on GitHub.

@davidmagill

This comment has been minimized.

davidmagill commented Mar 22, 2015

Are you suggesting that this should be the intended behavior? If so, I would ask the following:

If the setter method expects a string value, why should an empty cell pass it null? Also, I'm not sure how you would pass an empty string if that was required behavior...

The new behavior breaks backward compatibility.

@amolenaar amolenaar referenced this pull request Mar 22, 2015

Merged

Improve converters #613

@amolenaar amolenaar merged commit 2264d99 into unclebob:master Mar 31, 2015

amolenaar added a commit that referenced this pull request Mar 31, 2015

Merge pull request #687 from amolenaar/fix/686
Revert old behaviour for Slim String converter
@amolenaar

This comment has been minimized.

Collaborator

amolenaar commented Mar 31, 2015

Merged, as agreed by @antoine-aumjaud.

@amolenaar amolenaar deleted the amolenaar:fix/686 branch Mar 31, 2015

@amolenaar amolenaar added this to the Next release milestone Mar 31, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment