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

[GitHub] [lucene] jpountz opened a new pull request, #12366: Move TermAndBoost back to its original location.

jpountz opened a new pull request, #12366:
URL: https://github.com/apache/lucene/pull/12366

   PR #12169 accidentally moved the `TermAndBoost` class to a different location, which would break custom sub-classes of `QueryBuilder`. This commit moves it back to its original location.


-- 
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] iverase commented on a diff in pull request #12366: Move TermAndBoost back to its original location.

Posted by "iverase (via GitHub)" <gi...@apache.org>.
iverase commented on code in PR #12366:
URL: https://github.com/apache/lucene/pull/12366#discussion_r1228993657


##########
lucene/analysis/common/src/java/org/apache/lucene/analysis/synonym/word2vec/TermAndBoost.java:
##########
@@ -14,7 +14,9 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.lucene.util;
+package org.apache.lucene.analysis.synonym.word2vec;
+
+import org.apache.lucene.util.BytesRef;

Review Comment:
   I just noticed that `Word2VecSynonymProvider` is a public class the exposes `TermAndBoost` via the method `#getSynonyms`. So either we can make that method package private or we need to change the visibility of the class again. 



-- 
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] iverase commented on a diff in pull request #12366: Move TermAndBoost back to its original location.

Posted by "iverase (via GitHub)" <gi...@apache.org>.
iverase commented on code in PR #12366:
URL: https://github.com/apache/lucene/pull/12366#discussion_r1228445424


##########
lucene/analysis/common/src/java/org/apache/lucene/analysis/synonym/word2vec/TermAndBoost.java:
##########
@@ -14,7 +14,9 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.lucene.util;
+package org.apache.lucene.analysis.synonym.word2vec;
+
+import org.apache.lucene.util.BytesRef;

Review Comment:
   And even rename it so it does not get confused?



-- 
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] reta commented on pull request #12366: Move TermAndBoost back to its original location.

Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on PR #12366:
URL: https://github.com/apache/lucene/pull/12366#issuecomment-1589680899

   Thanks @jpountz , it actually did break (OpenSearch) and I suspect Elasticsearch as well


-- 
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] iverase commented on a diff in pull request #12366: Move TermAndBoost back to its original location.

Posted by "iverase (via GitHub)" <gi...@apache.org>.
iverase commented on code in PR #12366:
URL: https://github.com/apache/lucene/pull/12366#discussion_r1228443611


##########
lucene/analysis/common/src/java/org/apache/lucene/analysis/synonym/word2vec/TermAndBoost.java:
##########
@@ -14,7 +14,9 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.lucene.util;
+package org.apache.lucene.analysis.synonym.word2vec;
+
+import org.apache.lucene.util.BytesRef;

Review Comment:
   Should we make this class package private?



-- 
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] jpountz merged pull request #12366: Move TermAndBoost back to its original location.

Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz merged PR #12366:
URL: https://github.com/apache/lucene/pull/12366


-- 
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] jpountz commented on a diff in pull request #12366: Move TermAndBoost back to its original location.

Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on code in PR #12366:
URL: https://github.com/apache/lucene/pull/12366#discussion_r1228628622


##########
lucene/analysis/common/src/java/org/apache/lucene/analysis/synonym/word2vec/TermAndBoost.java:
##########
@@ -14,7 +14,9 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.lucene.util;
+package org.apache.lucene.analysis.synonym.word2vec;
+
+import org.apache.lucene.util.BytesRef;

Review Comment:
   Good call on reducing visibility. I tried to see if private worked and it did not, but I forgot to check if pkg-private would work, it does!
   
   I think it's fine to use the same name, at least I don't want to give it a weird name for the sake of having a different name? E.g. we have lots of `ScoreAndDoc` classes across many packages and it's fine?



-- 
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] iverase commented on a diff in pull request #12366: Move TermAndBoost back to its original location.

Posted by "iverase (via GitHub)" <gi...@apache.org>.
iverase commented on code in PR #12366:
URL: https://github.com/apache/lucene/pull/12366#discussion_r1228630485


##########
lucene/analysis/common/src/java/org/apache/lucene/analysis/synonym/word2vec/TermAndBoost.java:
##########
@@ -14,7 +14,9 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.lucene.util;
+package org.apache.lucene.analysis.synonym.word2vec;
+
+import org.apache.lucene.util.BytesRef;

Review Comment:
   it makes sense, thanks!



-- 
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] uschindler commented on a diff in pull request #12366: Move TermAndBoost back to its original location.

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #12366:
URL: https://github.com/apache/lucene/pull/12366#discussion_r1229079925


##########
lucene/analysis/common/src/java/org/apache/lucene/analysis/synonym/word2vec/TermAndBoost.java:
##########
@@ -14,7 +14,9 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.lucene.util;
+package org.apache.lucene.analysis.synonym.word2vec;
+
+import org.apache.lucene.util.BytesRef;

Review Comment:
   I would keep it public for now as it was before.



-- 
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