Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Removed the usage of libkiwix deprecated API #535

Merged
merged 3 commits into from Feb 10, 2022
Merged

Removed the usage of libkiwix deprecated API #535

merged 3 commits into from Feb 10, 2022

Conversation

veloman-yunkan
Copy link
Collaborator

Fixes #533

@veloman-yunkan
Copy link
Collaborator Author

@kelson42

In kiwix-tools, the deprecated libkiwix API was used only in kiwix-search and kiwix-reader.

kiwix-search is now clean.

Regarding kiwix-reader, do we need to keep it at all? Its name is misleading, since it is rather a variant of kiwix-search --suggest that works with some heuristic on old ZIM files without a title index.

@kelson42
Copy link
Contributor

kelson42 commented Feb 8, 2022

@veloman-yunkan It seems we can delete kiwix-reader and actually there is already a requea]st to remove kiwix-search now that all the logic in at the libzim level, see openzim/zim-tools#262.

@veloman-yunkan veloman-yunkan changed the title [WIP] Removed the usage of libkiwix deprecated API Removed the usage of libkiwix deprecated API Feb 9, 2022
@veloman-yunkan veloman-yunkan marked this pull request as ready for review February 9, 2022 11:32
mgautierfr
mgautierfr requested changes Feb 9, 2022
View reviewed changes
Copy link
Member

@mgautierfr mgautierfr left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove the redundant line at src/searcher/kiwix-search.cpp#L117 has shown by codefactor.io ?

Else we are good (No need to ask for a second review once it is fixed)

@veloman-yunkan
Cleared kiwix-search of libkiwix deprecated API
2bb080e
@veloman-yunkan
Goodbye kiwix-read
f9ce353 
`kiwix-read` was a confusing tool. It was a loose equivalent of
`kiwix-search --suggest` also able to work on old ZIM archives that didn't
contain a title index. Its existence is not justified.
@veloman-yunkan
Now kiwix-tools is built with werror=true
8b36e94
@veloman-yunkan veloman-yunkan force-pushed the saying_no_to_deprecated_libkiwix_api branch from d648583 to 8b36e94 Compare February 10, 2022 09:13
@veloman-yunkan
Copy link
Collaborator Author

Can you remove the redundant line at src/searcher/kiwix-search.cpp#L117 has shown by codefactor.io ?

Else we are good (No need to ask for a second review once it is fixed)

Done

@kelson42 kelson42 merged commit 2c701cb into master Feb 10, 2022
@kelson42 kelson42 deleted the saying_no_to_deprecated_libkiwix_api branch February 10, 2022 09:34
@kelson42 kelson42 mentioned this pull request Feb 27, 2022
Closed
@kelson42 kelson42 mentioned this pull request May 9, 2022
Closed
@kelson42 kelson42 added this to the 3.3.0 milestone Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgautierfr mgautierfr mgautierfr requested changes

@kelson42 kelson42 kelson42 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.

Remove usage of deprecated libkiwix API
3 participants
@veloman-yunkan @kelson42 @mgautierfr