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

freeCodeCamp builder #5

Merged
merged 81 commits into from
Jul 18, 2023
Merged

freeCodeCamp builder #5

merged 81 commits into from
Jul 18, 2023

Conversation

mdp
Copy link
Collaborator

@mdp mdp commented Apr 18, 2023

This is now working and includes:

  • A Dockerfile for building the zim file (see Readme)
  • A Makefile for fetching, preparing the curriculum, building the Vite app, and packaging it in a zim file
  • Support for 9 of the Javascript courses
  • Support for all the languages currently supported by FCC
  • Basic searching support in the zim file (This is an SPA, so this is handled as redirects)

@mdp mdp mentioned this pull request Apr 19, 2023
@mdp
Copy link
Collaborator Author

mdp commented Jun 16, 2023

@rgaudin I think there's just the remainin question about is_front and being able to search for Challenges, but all the other items should be resolved. Thanks for all your help on this. It's been several years since I've done Python (and then very little), sorry for the less than Pythonic code.

@rgaudin rgaudin self-requested a review June 19, 2023 15:39
Copy link
Member

@rgaudin rgaudin left a comment

Choose a reason for hiding this comment

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

Thank you @mdp ! I figured when I saw Path.py! Brought me back a few years back 😉

Most of the inline comments are suggestions towards our convention. Feel free to discard them.

I'd like us to check the CSS hack though. I think checking that the client was built before creating the ZIM would also help.

Regarding is_front ; I've explained inline. As there is a bug in libkiwix, we have to choose between suggestions and random ATM but we'll fix that.

Once this is merged, I'd like you to provide a list of what the scraper can and can't do (curriculum/courses). As suggested earlier, we need to know what options to pass --course.

I've noticed that most (if not all) of python-for-everybody fail to load with a “No description in challenge” exception (already mentioned). Their frontmatter doesn't include a description so that's the reason but they do have content and skipping the exception just reveals broken challenges.

If we are to create FCC ZIM files, we need to know the limits of this tool and at the moment it's not defined.

.github/workflows/qa.yaml Outdated Show resolved Hide resolved
openzim/fcctozim/build.py Outdated Show resolved Hide resolved
openzim/fcctozim/build.py Show resolved Hide resolved
openzim/fcctozim/build.py Show resolved Hide resolved
openzim/fcctozim/build.py Outdated Show resolved Hide resolved
openzim/fcctozim/prebuild.py Outdated Show resolved Hide resolved
openzim/fcctozim/build.py Outdated Show resolved Hide resolved
openzim/fcctozim/build.py Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
client/src/style.css Outdated Show resolved Hide resolved
@rgaudin
Copy link
Member

rgaudin commented Jul 4, 2023

Just saw another glitch ; in python-for-everybody, a number of challenges like networking-text-processing have titles that include a colon : which breaks the YAML parsing.

See /fcc/curriculum/english/scientific-computing-with-python/python-for-everybody/networking-text-processing.md:

---
id: 5e7b9f0c0b6c005b0e76f074
title: 'Networking: Text Processing'
challengeType: 11
videoId: Pv_pJgVu8WI
bilibiliIds:
  aid: 804442498
  bvid: BV16y4y1j7WW
  cid: 377329124
dashedName: networking-text-processing
---

The rendered title is 'Networking

Screenshot 2023-07-04 at 09 44 45

mdp added a commit that referenced this pull request Jul 13, 2023
- Update action to 'main' branch - Resolves: #5 (comment)
- Fix schema change issue with FCC. Challenge order is now a list
of dictionaries
- Linter fix
- Fix publisher and creator attributes - Resolves: #5 (comment)
and #5 (comment) and
#5 (comment)
- is_front on redirects only. Resolves: #5 (comment)
-
- Update action to 'main' branch - Resolves: #5 (comment)
- Fix schema change issue with FCC. Challenge order is now a list
of dictionaries
- Linter fix
- Fix publisher and creator attributes - Resolves: #5 (comment)
and #5 (comment) and
#5 (comment)
- is_front on redirects only. Resolves: #5 (comment)
-
@benoit74
Copy link
Collaborator

I would like to thank you @mdp, this is really great work I think. The last mile seems almost here now.

I appreciate especially the fact that this scrapper proves it is totally feasible / easy to host a Vite (Vue.JS here, but probably does not matter much indeed) application inside a Zim. We briefly discussed about it with @kelson42 and @rgaudin few days ago for the UI enhancements of kolibri, for which I was tempted to migrate to Vue.JS for simpler/smaller UI code once the UI becomes a bit dynamic/complex. I believe that you gave a good answer to my interrogations ^^

Copy link
Member

@rgaudin rgaudin left a comment

Choose a reason for hiding this comment

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

Those changes are all good. Looking at the unresolved discussions now

@mdp
Copy link
Collaborator Author

mdp commented Jul 18, 2023

Sorry, just now getting back to this. Tackling the current build break, which looks like a minor directory issue.

@mdp
Copy link
Collaborator Author

mdp commented Jul 18, 2023

@rgaudin With regards to the colon issue I need to play around with Python's YAML parsing and figure out if there's an easy fix on this one. I'll try to have this one fixed today as well as the tests.

@rgaudin
Copy link
Member

rgaudin commented Jul 18, 2023

@rgaudin With regards to the colon issue I need to play around with Python's YAML parsing and figure out if there's an easy fix on this one. I'll try to have this one fixed today as well as the tests.

Yeah I want to merge this now (as soon as I fix the tests) and open a ticket for the colon issue. Does that work for you?

@mdp
Copy link
Collaborator Author

mdp commented Jul 18, 2023

I just pushed a build fix. Merge sounds good. I'll start on the colon issue now.

@rgaudin rgaudin merged commit 11c6ae6 into main Jul 18, 2023
3 checks passed
@rgaudin rgaudin deleted the mdp/rework branch July 18, 2023 16:02
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.

5 participants