Merges the changes from the release/3.0 branch to release/4.0 branch
#345
Conversation
|
#321 is facet deep paging |
|
@nzdev Oh yes right. Could you then take a look at the LuceneSearchExecutor changes in this PR and see if it is merged correctly? - when testing all the tests succeed😁 |
|
Here's what I'm thinking. Steps:
|
|
Looks like conflicts need to be resolved |
| //protected override void Dispose(bool disposing) | ||
| //{ | ||
| //if (!_disposedValue) | ||
| //{ | ||
| // if (disposing) | ||
| // { | ||
| // _searcherManager.Dispose(); | ||
| // } | ||
|
|
||
| // _disposedValue = true; | ||
| //} | ||
| //base.Dispose(disposing); | ||
| //} |
There was a problem hiding this comment.
This will replace the new virtual when possible to align with the dispose pattern.
There was a problem hiding this comment.
Looks like this would break any classes overriding public virtual void Dispose(). We should revert to keep compatibility and mark as obsolete in V5.
There was a problem hiding this comment.
This code is commented out I can't see it being problematic before uncommented?
| public virtual FacetsConfig GetDefaultFacetConfig() => new FacetsConfig(); | ||
|
|
||
| /// <summary> | ||
| /// Disposes of the searcher | ||
| /// </summary> | ||
| /// <param name="disposing"></param> | ||
| protected virtual void Dispose(bool disposing) | ||
| { | ||
| return new FacetsConfig(); | ||
| if (!_disposedValue) | ||
| { | ||
| _disposedValue = true; | ||
| } | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public virtual void Dispose() | ||
| /// <inheritdoc /> | ||
| public void Dispose() | ||
| { | ||
|
|
||
| // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method | ||
| Dispose(disposing: true); | ||
| GC.SuppressFinalize(this); |
There was a problem hiding this comment.
Align with the dispose pattern
There was a problem hiding this comment.
Looks like this would break any classes overriding public virtual void Dispose(). We should revert to keep compatibility and mark as obsolete in V5.
There was a problem hiding this comment.
@nzdev I've readded the virtual modifier and added an Obsolete attribute describing that you should override the Dispose(bool) instead.
|
@Shazwazza This is now updated. I've fixed all warnings that isn't part of the |
|
RS0036 should still be active. The unshipped API .txts files should get deleted and regenerated now that nullable / API should be ready. Will review rest later |
|
@nzdev super then we can remove the nowarn |
@nzdev I tried generating the files but couldn't seem to get all the warnings to go away may be I'm doing something incorrectly. But I think the <!-- Disable the nullable warnings when compiling for .NET Standard 2.0 -->
<PropertyGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
<NoWarn>$(NoWarn);nullable;RS0037</NoWarn>
</PropertyGroup>
<ItemGroup Condition="'$(MSBuildProjectName)' != 'Examine.Web.Demo' AND '$(MSBuildProjectName)' != 'Examine.Test'">
<AdditionalFiles Include="PublicAPI/$(TargetFramework)/PublicAPI.Shipped.txt" />
<AdditionalFiles Include="PublicAPI/$(TargetFramework)/PublicAPI.Unshipped.txt" />
</ItemGroup>I think we need to ignore |
|
#349 addresses the public API files and fixes / suppresses most of the warnings. |
|
#349 should include everything from this one so i'll close this one |
This merges the changes in the
release/3.0branch to therelease/4.0branch. This primarily contains #320.@nzdev Is there any way we can use faceting and the feature in #320 at the same time? I couldn't see an obvious way to accomplice this.
And @nzdev What would be a good XML comment for
ShardIndexin thesrc/Examine.Lucene/Search/LuceneSearchResult.csfile?