You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "mikemccand (via GitHub)" <gi...@apache.org> on 2023/10/18 15:52:23 UTC

[I] Don't provide two ways to build an FST [lucene]

mikemccand opened a new issue, #12695:
URL: https://github.com/apache/lucene/issues/12695

   ### Description
   
   Spinoff from [this great comment](https://github.com/apache/lucene/pull/12633#discussion_r1360866889):
   
   Today, you can build an FST either using `FSTCompiler.Builder` or the two-parameter `FSTCompiler` ctor.
   
   Should we remove the latter and expose only the Builder?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [I] Don't provide two ways to build an FST [lucene]

Posted by "cavorite (via GitHub)" <gi...@apache.org>.
cavorite commented on issue #12695:
URL: https://github.com/apache/lucene/issues/12695#issuecomment-1774229048

   I'm be willing to work on this issues (as a way to get more familiar with Lucene's internal code base). First, I'd like to see if I'm understanding the work needed. 
   
   So far, it seems that the refactoring is relatively straightforward to do: replace the invocations of the constructor of `FSTCompiler` by creating an instance of `Builder` and then calling the `build()` method.  For instance, replacing the line:
   
   ```
   FSTCompiler<BytesRef> fstCompiler = new FSTCompiler<>(FST.INPUT_TYPE.BYTE4, outputs);
   ```
   
   by this one:
   
   ```
   FSTCompiler<BytesRef> fstCompiler = new FSTCompiler.Builder<>(FST.INPUT_TYPE.BYTE4, outputs).build();
   ```
   
   There are about 40 usages of the constructor in the repositoy (including modules other than `core`), so we need to replace the usages in all of them and also mark the constructor as deprecated (so that it can be deleted in an upcoming major release).
   
   Would that be all what's necessary for this issue?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [I] Don't provide two ways to build an FST [lucene]

Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand closed issue #12695: Don't provide two ways to build an FST
URL: https://github.com/apache/lucene/issues/12695


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [I] Don't provide two ways to build an FST [lucene]

Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on issue #12695:
URL: https://github.com/apache/lucene/issues/12695#issuecomment-1774232380

   Yes that's exactly the idea!  Thank you @cavorite for tackling this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [I] Don't provide two ways to build an FST [lucene]

Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on issue #12695:
URL: https://github.com/apache/lucene/issues/12695#issuecomment-1819661531

   Backported to 9.9.0 -- closing.  Thank you @cavorite!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org