Just brainstorming here. This contract has always been an issue because the analyzers warn if the protected virtual void Dispose(bool disposing) signature is not used. But until now it was clear that this contract was required to pass the waitForMerges state through to subclasses. Is there a reasonable way we can remove this parameter and restore the original signature before the release, or are we stuck because IndexWriter still needs this to be passed through?
In other words, can we keep waitForMerges out of the public API that inheritors of this class use?
Originally posted by @NightOwl888 in #1311 (comment)
IndexWriter exposes its dispose hook as protected virtual void Dispose(bool disposing, bool waitForMerges) instead of the conventional protected virtual void Dispose(bool disposing), a LuceneNet-specific choice made so the public Dispose() and [Obsolete] Dispose(bool waitForMerges) overloads could thread waitForMerges down to the close path. This diverges from the dispose pattern the analyzers expect (hence the several [SuppressMessage] suppressions on the method) and, since it's a protected virtual member, changing it becomes a breaking change once we ship, so it should be fixed before the next release. We can restore the standard Dispose(bool disposing) signature and keep waitForMerges off the inheritance surface, since both public call sites already know the flag and can pass it into the internal shutdown path directly rather than through the virtual hook (the only tradeoff, that a subclass can't influence the waitForMerges: false shutdown, is on the [Obsolete] path; upstream Java did allow overriding close(boolean), but no in-tree subclass relies on that today). Note that the waitForMerges support was removed in Lucene 5.0.
Originally posted by @NightOwl888 in #1311 (comment)
IndexWriterexposes its dispose hook asprotected virtual void Dispose(bool disposing, bool waitForMerges)instead of the conventionalprotected virtual void Dispose(bool disposing), a LuceneNet-specific choice made so the publicDispose()and[Obsolete] Dispose(bool waitForMerges)overloads could threadwaitForMergesdown to the close path. This diverges from the dispose pattern the analyzers expect (hence the several[SuppressMessage]suppressions on the method) and, since it's aprotected virtualmember, changing it becomes a breaking change once we ship, so it should be fixed before the next release. We can restore the standardDispose(bool disposing)signature and keepwaitForMergesoff the inheritance surface, since both public call sites already know the flag and can pass it into the internal shutdown path directly rather than through the virtual hook (the only tradeoff, that a subclass can't influence thewaitForMerges: falseshutdown, is on the[Obsolete]path; upstream Java did allow overridingclose(boolean), but no in-tree subclass relies on that today). Note that thewaitForMergessupport was removed in Lucene 5.0.