-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Conversation
There was a problem hiding this 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
.
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. |
* Implement version field type * Fix failing test * Add skip version to test
* Implement version field type * Fix failing test * Add skip version to test
* 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>
@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 thatVersion
becomesDocVersion
?