You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/11/30 01:20:56 UTC
[GitHub] [lucene] rmuir opened a new pull request #491: LUCENE-10270: Improve MIGRATE.md
rmuir opened a new pull request #491:
URL: https://github.com/apache/lucene/pull/491
The idea here is to do an 80/20 pass on MIGRATE.md to make it more readable, searchable, less ambiguous.
* Separate sections for 9.0 and 9.1
* Remove abbreviations for artifact, package, class names etc. e.g. `lucene-core` instead of `core` and `org.apache.lucene.analysis` instead of `o.a.l.a`.
* Specify "java" for text blocks to get syntax highlighting
* When provided, consistently put JIRA issue in the same place
* Fixed-width font for classes/reserved words (e.g. false, true, long, makes for less ambiguous reading)
* More use of tables vs lists when there is mapping of old -> new names (packages, classes, etc)
* Use consistent notation for method calls (Class.method() vs Class.method vs Class#method etc)
--
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
[GitHub] [lucene] rmuir merged pull request #491: LUCENE-10270: Improve MIGRATE.md
Posted by GitBox <gi...@apache.org>.
rmuir merged pull request #491:
URL: https://github.com/apache/lucene/pull/491
--
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
[GitHub] [lucene] rmuir commented on pull request #491: LUCENE-10270: Improve MIGRATE.md
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #491:
URL: https://github.com/apache/lucene/pull/491#issuecomment-983140470
Sorry for spamming reviewers, if anyone doesn't mind taking a look at this simple doc PR. I know it isn't perfect, I am just trying to improve. But i find myself working on other issues that want to touch this file too. Because it is a conflict-beast, it would be good to get it in sooner than later.
--
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
[GitHub] [lucene] msokolov commented on a change in pull request #491: LUCENE-10270: Improve MIGRATE.md
Posted by GitBox <gi...@apache.org>.
msokolov commented on a change in pull request #491:
URL: https://github.com/apache/lucene/pull/491#discussion_r759769058
##########
File path: lucene/MIGRATE.md
##########
@@ -156,42 +179,42 @@ For example, these entries are not allowed at all and an exception is thrown whe
日本経済新聞,日経 新聞,ニッケイ シンブン,カスタム名詞
```
-## JapaneseTokenizer no longer emits original (compound) tokens by default when the mode is not NORMAL (LUCENE-9123)
+### JapaneseTokenizer no longer emits original (compound) tokens by default when the mode is not NORMAL (LUCENE-9123)
-JapaneseTokenizer and JapaneseAnalyzer no longer emits original tokens when discardCompoundToken option is not specified.
-The constructor option has been introduced since Lucene 8.5.0, and the default value is changed to true.
+`JapaneseTokenizer` and `JapaneseAnalyzer` no longer emits original tokens when `discardCompoundToken` option is not specified.
+The constructor option has been introduced since Lucene 8.5.0, and the default value is changed to `true`.
When given the text "株式会社", JapaneseTokenizer (mode != NORMAL) emits decompounded tokens "株式" and "会社" only and no
-longer outputs the original token "株式会社" by default. To output original tokens, discardCompoundToken option should be
-explicitly set to false. Be aware that if this option is set to false SynonymFilter or SynonymGraphFilter does not work
+longer outputs the original token "株式会社" by default. To output original tokens, `discardCompoundToken` option should be
+explicitly set to `false`. Be aware that if this option is set to `false`, `SynonymFilter` or `SynonymGraphFilter` does not work
correctly (see LUCENE-9173).
-## Analysis factories now have customizable symbolic names (LUCENE-8778) and need additional no-arg constructor (LUCENE-9281)
+### Analysis factories now have customizable symbolic names (LUCENE-8778) and need additional no-arg constructor (LUCENE-9281)
-The SPI names for concrete subclasses of TokenizerFactory, TokenFilterFactory, and CharfilterFactory are no longer
+The SPI names for concrete subclasses of `TokenizerFactory`, `TokenFilterFactory`, and `CharfilterFactory` are no longer
derived from their class name. Instead, each factory must have a static "NAME" field like this:
-```
+```java
/** o.a.l.a.standard.StandardTokenizerFactory's SPI name */
public static final String NAME = "standard";
```
-A factory can be resolved/instantiated with its NAME by using methods such as TokenizerFactory#lookupClass(String)
-or TokenizerFactory#forName(String, Map<String,String>).
+A factory can be resolved/instantiated with its `NAME` by using methods such as `TokenizerFactory.lookupClass(String)`
+or `TokenizerFactory.forName(String, Map<String,String>)`.
-If there are any user-defined factory classes that don't have proper NAME field, an exception will be thrown
-when (re)loading factories. e.g., when calling TokenizerFactory#reloadTokenizers(ClassLoader).
+If there are any user-defined factory classes that don't have proper `NAME` field, an exception will be thrown
+when (re)loading factories. e.g., when calling `TokenizerFactory.reloadTokenizers(ClassLoader)`.
In addition starting all factories need to implement a public no-arg constructor, too. The reason for this
change comes from the fact that Lucene now uses `java.util.ServiceLoader` instead its own implementation to
load the factory classes to be compatible with Java Module System changes (e.g., load factories from modules).
In the future, extensions to Lucene developed on the Java Module System may expose the factories from their
`module-info.java` file instead of `META-INF/services`.
-This constructor is never called by Lucene, so by default it throws a UnsupportedOperationException. User-defined
+This constructor is never called by Lucene, so by default it throws a `UnsupportedOperationException`. User-defined
Review comment:
nit: "an" Unsupported...
##########
File path: lucene/MIGRATE.md
##########
@@ -200,45 +223,42 @@ factory classes should implement it in the following way:
(`defaultCtorException()` is a protected static helper method)
-## TermsEnum is now fully abstract (LUCENE-8292)
-
-TermsEnum has been changed to be fully abstract, so non-abstract subclass must implement all it's methods.
-Non-Performance critical TermsEnums can use BaseTermsEnum as a base class instead. The change was motivated
-by several performance issues with FilterTermsEnum that caused significant slowdowns and massive memory consumption due
-to not delegating all method from TermsEnum. See LUCENE-8292 and LUCENE-8662
+### TermsEnum is now fully abstract (LUCENE-8292, LUCENE-8662)
-## RAMDirectory, RAMFile, RAMInputStream, RAMOutputStream removed
+`TermsEnum` has been changed to be fully abstract, so non-abstract subclass must implement all it's methods.
Review comment:
nit: "non-abstract subclasses must implement all of its methods."
##########
File path: lucene/MIGRATE.md
##########
@@ -156,42 +179,42 @@ For example, these entries are not allowed at all and an exception is thrown whe
日本経済新聞,日経 新聞,ニッケイ シンブン,カスタム名詞
```
-## JapaneseTokenizer no longer emits original (compound) tokens by default when the mode is not NORMAL (LUCENE-9123)
+### JapaneseTokenizer no longer emits original (compound) tokens by default when the mode is not NORMAL (LUCENE-9123)
-JapaneseTokenizer and JapaneseAnalyzer no longer emits original tokens when discardCompoundToken option is not specified.
-The constructor option has been introduced since Lucene 8.5.0, and the default value is changed to true.
+`JapaneseTokenizer` and `JapaneseAnalyzer` no longer emits original tokens when `discardCompoundToken` option is not specified.
+The constructor option has been introduced since Lucene 8.5.0, and the default value is changed to `true`.
When given the text "株式会社", JapaneseTokenizer (mode != NORMAL) emits decompounded tokens "株式" and "会社" only and no
-longer outputs the original token "株式会社" by default. To output original tokens, discardCompoundToken option should be
-explicitly set to false. Be aware that if this option is set to false SynonymFilter or SynonymGraphFilter does not work
+longer outputs the original token "株式会社" by default. To output original tokens, `discardCompoundToken` option should be
+explicitly set to `false`. Be aware that if this option is set to `false`, `SynonymFilter` or `SynonymGraphFilter` does not work
correctly (see LUCENE-9173).
-## Analysis factories now have customizable symbolic names (LUCENE-8778) and need additional no-arg constructor (LUCENE-9281)
+### Analysis factories now have customizable symbolic names (LUCENE-8778) and need additional no-arg constructor (LUCENE-9281)
-The SPI names for concrete subclasses of TokenizerFactory, TokenFilterFactory, and CharfilterFactory are no longer
+The SPI names for concrete subclasses of `TokenizerFactory`, `TokenFilterFactory`, and `CharfilterFactory` are no longer
derived from their class name. Instead, each factory must have a static "NAME" field like this:
-```
+```java
/** o.a.l.a.standard.StandardTokenizerFactory's SPI name */
public static final String NAME = "standard";
```
-A factory can be resolved/instantiated with its NAME by using methods such as TokenizerFactory#lookupClass(String)
-or TokenizerFactory#forName(String, Map<String,String>).
+A factory can be resolved/instantiated with its `NAME` by using methods such as `TokenizerFactory.lookupClass(String)`
+or `TokenizerFactory.forName(String, Map<String,String>)`.
-If there are any user-defined factory classes that don't have proper NAME field, an exception will be thrown
-when (re)loading factories. e.g., when calling TokenizerFactory#reloadTokenizers(ClassLoader).
+If there are any user-defined factory classes that don't have proper `NAME` field, an exception will be thrown
+when (re)loading factories. e.g., when calling `TokenizerFactory.reloadTokenizers(ClassLoader)`.
In addition starting all factories need to implement a public no-arg constructor, too. The reason for this
change comes from the fact that Lucene now uses `java.util.ServiceLoader` instead its own implementation to
load the factory classes to be compatible with Java Module System changes (e.g., load factories from modules).
In the future, extensions to Lucene developed on the Java Module System may expose the factories from their
`module-info.java` file instead of `META-INF/services`.
-This constructor is never called by Lucene, so by default it throws a UnsupportedOperationException. User-defined
+This constructor is never called by Lucene, so by default it throws a `UnsupportedOperationException`. User-defined
factory classes should implement it in the following way:
-```
+```java
Review comment:
nice!
--
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
[GitHub] [lucene] rmuir commented on pull request #491: LUCENE-10270: Improve MIGRATE.md
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #491:
URL: https://github.com/apache/lucene/pull/491#issuecomment-983177549
@msokolov Thanks for the review and the bugs found. I pushed another commit.
--
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