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

Separate addition of default and non-default value comparers. #1257

Merged
merged 2 commits into from Sep 3, 2018

Conversation

midgleyc
Copy link
Contributor

Currently, replacing only the default value comparer while keeping all others the same involves rather a lot of code:

var existingComparers = Service.Instance.ValueComparers.ToList();
foreach (var existingComparer in existingComparers)
{
	Service.Instance.UnregisterValueComparer(existingComparer);
}

Service.Instance.RegisterValueComparer(new NullConversionDefaultValueComparer());
foreach (var existingComparer in existingComparers.Where(x => !(x is DefaultValueComparer)))
{
	Service.Instance.RegisterValueComparer(existingComparer);
}

This change separates the handling of default and non-default value comparers, and stops it splitting on the type.

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added an entry to the changelog

@SabotageAndi
Copy link
Contributor

@midgleyc Thanks for the PR.
Please add an entry to the changelog and I will merge the PR. Thanks!

@midgleyc
Copy link
Contributor Author

midgleyc commented Sep 3, 2018

I've added the entry, cheers.

@SabotageAndi SabotageAndi added this to the SpecFlow 3.0 milestone Sep 3, 2018
@SabotageAndi SabotageAndi merged commit 8c5a1af into SpecFlowOSS:master Sep 3, 2018
SabotageAndi pushed a commit that referenced this pull request Oct 1, 2018
…1277)

Following on from #1257, this pull request puts in a slightly more sensible contract of only having one default at any time, and takes advantage of the newly added generics.

I've deleted the old implementation as it was never part of a released version, so it didn't seem to make much sense to include it (same for the obsoleted method). I can add it back if you'd like.

I'm not completely sold on the "SetDefault" naming.

It's a breaking change for the existing functionality, but not since the last release.

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue).
- [ ] New feature (non-breaking change which adds functionality).
- [x] Breaking change (fix or feature that would cause existing functionality to not work as expected).

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [x] I've added tests for my code.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have added an entry to the changelog
@midgleyc midgleyc deleted the DefaultValueComparer branch December 4, 2018 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants