Skip to content

Implement version field type #5200

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

Merged
merged 3 commits into from
Jan 7, 2021
Merged

Implement version field type #5200

merged 3 commits into from
Jan 7, 2021

Conversation

stevejgordon
Copy link
Contributor

@Mpdreamz I've attempted this by reviewing previous PRs and a bit of sleuthing! Please let me know if I've missed anything. I've been able to use the mapping on a test project.

Main challenge in naming in FieldCapabilities since that already has the concept of the "_version" field. I welcome opinions on the naming. Perhaps in 8.0 we can consider renaming both of these so that Version becomes DocVersion?

Copy link
Member

@Mpdreamz Mpdreamz left a comment

Choose a reason for hiding this comment

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

Almost there one failing test on parity:

[xUnit.net 00:00:24.7659600]     Tests.Mapping.PropertyDescriptorTests.IPropertiesDescriptorImplementsAllPropertyMethodsOfPropertiesDescriptor [FAIL]
  Failed Tests.Mapping.PropertyDescriptorTests.IPropertiesDescriptorImplementsAllPropertyMethodsOfPropertiesDescriptor [25 ms]
  Error Message:
   Expected concreteMethodNames.Except(interfaceMethodNames) to be empty, but found {"Version"}.
  Stack Trace:
     at FluentAssertions.Execution.XUnit2TestFramework.Throw(String message)
   at FluentAssertions.Execution.TestFrameworkProvider.Throw(String message)
   at FluentAssertions.Execution.DefaultAssertionStrategy.HandleFailure(String message)
   at FluentAssertions.Execution.AssertionScope.FailWith(Func`1 failReasonFunc)
   at FluentAssertions.Execution.AssertionScope.FailWith(Func`1 failReasonFunc)
   at FluentAssertions.Execution.AssertionScope.FailWith(String message, Object[] args)
   at FluentAssertions.Execution.GivenSelector`1.FailWith(String message, Object[] args)
   at FluentAssertions.Execution.GivenSelector`1.FailWith(String message, Func`2[] args)
   at FluentAssertions.Collections.CollectionAssertions`2.BeEmpty(String because, Object[] becauseArgs)
   at Tests.Mapping.PropertyDescriptorTests.IPropertiesDescriptorImplementsAllPropertyMethodsOfPropertiesDescriptor() in D:\a\1\s\tests\Tests\Mapping\PropertyDescriptorTests.cs:line 42

FieldTypes.Version is indeed a conundrum. Don't think a clean option exists. ++ to DocVersion in 8 for _version.

@stevejgordon
Copy link
Contributor Author

Thanks @Mpdreamz. I've fixed up the interface causing the failure. Will allow the tests to run in CI and then merge if that's all good.

@stevejgordon stevejgordon merged commit 28a3ac7 into 7.11 Jan 7, 2021
@stevejgordon stevejgordon deleted the feature/version-field branch January 7, 2021 13:17
github-actions bot pushed a commit that referenced this pull request Jan 7, 2021
* Implement version field type
* Fix failing test
* Add skip version to test
github-actions bot pushed a commit that referenced this pull request Jan 7, 2021
* Implement version field type
* Fix failing test
* Add skip version to test
stevejgordon added a commit that referenced this pull request Jan 7, 2021
* Implement version field type
* Fix failing test
* Add skip version to test

Co-authored-by: Steve Gordon <sgordon@hotmail.co.uk>
stevejgordon added a commit that referenced this pull request Jan 11, 2021
* Implement version field type (#5200)

* Implement version field type
* Fix failing test
* Add skip version to test

* Fixup namespace for master

Co-authored-by: Steve Gordon <sgordon@hotmail.co.uk>
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.

2 participants