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.

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

Move scraper to subfolder and initialize new Vue.js project #207

Merged
merged 5 commits into from
May 28, 2024

Conversation

dan-niles
Copy link
Collaborator

@dan-niles dan-niles commented May 27, 2024

Changes:

  • Scraper (Python code) has been moved to the scraper subfolder
  • Vue.JS is now used as main UI framework; all of its code is in the zimui subfolder; it is rendered with Vite to produce a static website.
  • Vuetify has been installed and setup
  • QA and Tests workflows have been adapted
    • to the new folder structure
    • to also QA and Test the Vue.JS part
  • precommit hooks have been configured for the Vue.JS part
  • Dockerfile has been adapted to first build the Vue.JS part in a dedicated stage and then embed the generated files into the final Python-based image
  • New CLI argument --zimui-dist has been added to specify the folder where zimui has been built
  • Switched to "on-the-fly" items addition to the ZIM by using Creator instead of make_zim_file

Disclaimer: From this point, the scraper is intentionally quite broken in the sense that it does not create a usable ZIM anymore since we had to comment out significant code portions to move to the new ZIM UI and Creator.

Closes #205

@dan-niles dan-niles self-assigned this May 27, 2024
@dan-niles dan-niles marked this pull request as ready for review May 27, 2024 17:38
@dan-niles dan-niles requested a review from benoit74 May 27, 2024 17:38
Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

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

Few comments inline.

Some other points:

  • you must mention in your issue that:
    • you've started the move to "on-the-fly" items addition to the ZIM (I've just opened Use scraperlib & pylibzim properly - add items to the ZIM "on the fly" #209 which is about it, the move is not yet finished since you have to fix all commented code)
    • from this point, the scraper is intentionally quite broken in the sense that it does not create a usable ZIM anymore since we had to comment out significant code portions to move to the new ZIM ui and creator
  • add a disclaimer at the beginning of README.md explaining that for now the scraper is undergoing significant rework for 3.0 release and the main branch is not very stable ; v2 branch is available (I just created it) should an important patch be pushed before v3 release
  • do we really need to keep the zimui/README.md, it does not provide much value from my PoV ?

We should also open new issues to not forget that we have to (no need to do it in current PR, can be tackled afterwards):

  • add a CONTRIBUTING.md at the root of the project explaining how to develop the zimui, just like we have at https:/openzim/kolibri/blob/new_ui/CONTRIBUTING.md
  • fix code coverage upload for Python part ; looking at the CI it looks like it fails to find the coverage report, it is probably broken as well in kolibri ; since we have almost zero test anyway, it is not an urgent problem to solve ^^

scraper/src/youtube2zim/entrypoint.py Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@dan-niles
Copy link
Collaborator Author

Updated the README.md with a disclaimer and removed the README in the zimui folder in 7431881

@dan-niles dan-niles requested a review from benoit74 May 28, 2024 13:41
Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@benoit74 benoit74 merged commit 43b0fa2 into main May 28, 2024
7 checks passed
@benoit74 benoit74 deleted the new_ui_init branch May 28, 2024 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move scraper to subfolder and initialize new Vue.js project
2 participants