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 2022/08/29 21:38:09 UTC
[GitHub] [lucene] mdmarshmallow opened a new pull request, #11729: LUCENE-11728: Improve code clarity for OrdinalMap
mdmarshmallow opened a new pull request, #11729:
URL: https://github.com/apache/lucene/pull/11729
### Description
<!--
If this is your first contribution to Lucene, please make sure you have reviewed the contribution guide.
https://github.com/apache/lucene/blob/main/CONTRIBUTING.md
-->
* Added comments to `OrdinalMap` to hopefully make the code and map construction algorithm clearer
* Changed some variable names
* Replaced `firstSegmentBits` with `firstSegmentHasAllTerms` boolean, the latter makes the code a bit more clear
--
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: [PR] GITHUB#11728: Improve code clarity for OrdinalMap [lucene]
Posted by "mdmarshmallow (via GitHub)" <gi...@apache.org>.
mdmarshmallow commented on PR #11729:
URL: https://github.com/apache/lucene/pull/11729#issuecomment-1881511709
Hi @jpountz, thanks for pushing! I took a look back at the comment and I think it references some variable names changes that were added with some of the code changes that weren't included. I'll make a follow up commit in the next few days to address some of those inconsistencies.
--
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: [PR] GITHUB#11728: Improve code clarity for OrdinalMap [lucene]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11729:
URL: https://github.com/apache/lucene/pull/11729#issuecomment-1880904411
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!
--
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] mdmarshmallow commented on pull request #11729: LUCENE-11728: Improve code clarity for OrdinalMap
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on PR #11729:
URL: https://github.com/apache/lucene/pull/11729#issuecomment-1242374535
That's fair, I think I also was getting a little too specific with the comments in some places. I'll go through and see if I can make the comments more general than they currently are.
--
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] stefanvodita commented on pull request #11729: GITHUB#11728: Improve code clarity for OrdinalMap
Posted by "stefanvodita (via GitHub)" <gi...@apache.org>.
stefanvodita commented on PR #11729:
URL: https://github.com/apache/lucene/pull/11729#issuecomment-1472199332
Bumping this PR because I think it’s a good change in its current form. The increased expressiveness would be helpful to me if I was using this class.
--
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: [PR] GITHUB#11728: Improve code clarity for OrdinalMap [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz closed pull request #11729: GITHUB#11728: Improve code clarity for OrdinalMap
URL: https://github.com/apache/lucene/pull/11729
--
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: [PR] GITHUB#11728: Improve code clarity for OrdinalMap [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on PR #11729:
URL: https://github.com/apache/lucene/pull/11729#issuecomment-1880971838
Hey @mdmarshmallow. There have been some changes in the meantime that introduced many conflicts, so what I did is that I took your main comment that describes the OrdinalMap algorithm and pushed it under your name. Sorry for the delay.
--
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 pull request #11729: LUCENE-11728: Improve code clarity for OrdinalMap
Posted by GitBox <gi...@apache.org>.
jpountz commented on PR #11729:
URL: https://github.com/apache/lucene/pull/11729#issuecomment-1242116139
This class is indeed a bit subtle, but most of the comments that you are suggesting seem to be paraphrasing what the code is doing and I'm not sure that they would actually help someone who would try to get more familiar with this class.
--
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] mdmarshmallow commented on pull request #11729: LUCENE-11728: Improve code clarity for OrdinalMap
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on PR #11729:
URL: https://github.com/apache/lucene/pull/11729#issuecomment-1247251315
Removed some of the comments that were maybe just paraphrasing the code too much. I also renamed some variables to what I think are more clear names. Hopefully the comments/variable names make more sense now?
--
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: [PR] GITHUB#11728: Improve code clarity for OrdinalMap [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on PR #11729:
URL: https://github.com/apache/lucene/pull/11729#issuecomment-1881513260
Oops, sorry about that. Thank you!
--
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