-
Notifications
You must be signed in to change notification settings - Fork 962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Writing a HOWTO migrate codec version #12930
base: main
Are you sure you want to change the base?
Conversation
dev-docs/codec-version-bump-howto.md
Outdated
* Create RW forks of the required `backwards_codecs` components (otherwise we cannot test writing to `backwards_codecs`) for use in | ||
backwards compatibility unit tests (the ones moved to backward_codecs). Copy previous implementations of the RW classes and update with the | ||
formats used in that specific codec. Use the RW components in test cases of relevant codecs. | ||
* Create a new `test.org.apache.lucene.backward_codecs.luceneXXX` package and copy in unit tests for any forked codec components. Modify |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this step of new java package creation moved up. Not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted the unit tests logically separated from the rest of the upgrade since it involves some unrelated changes. Where do you think it should go?
Co-authored-by: Shubham Chaudhary <36742242+shubhamvishu@users.noreply.github.com>
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! |
dev-docs/codec-version-bump-howto.md
Outdated
|
||
### Move original files to backwards_codecs | ||
* Deprecate previous Codec: | ||
* Create a new package for the previous codec of version XXX: `org.apache.lucene.backward_codecs.luceneXXX.LuceneXXXCodec`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we creating a new package or you mean a new class eg: LuceneXXXCodec
with old codec as specified here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I meant class, will change.
dev-docs/codec-version-bump-howto.md
Outdated
|
||
### Fork and modify files | ||
* Creating a new Codec: | ||
* Create a new package for your new codec of version XXX: `org.apache.lucene.codecs.luceneXXX`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about writing this how-to with the idea of changing the codec version from LuceneXY
to LuceneXZ
(?), that way I believe it'll be more clear to understand which codec is referenced in as opposed to writing LuceneXXX
or saying old vs new. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea! Will do this now. Also sorry for the delay, this work fell off my radar. Picking it back up.
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 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! |
More detail in #12918.
Changing PFOR encoding to FOR for doc blocks in #12741, required bumping the codec version. The codec upgrade process has numerous moving parts and it required multiple follow-ups to add lost unit tests.
Hopefully the lessons learned from this codec version "bump" will help others for the next time, whenever that may be.
Starting this is a draft PR since I am not sure that I am using the correct Lucene terminology, best practices, style, language, etc.
Suggestions and edits welcome :D
Closes #12918.