Initialized repository from template

[josephlewis42]

Initialized repository from the Python bootstrap template here: https://github.com/openzim/_python-bootstrap.

Part of #1

[benoit74]

Very good, few things to fix but already mostly OK, thank you!

A scraper is supposed to support only one Python version (3.12 as of now) since it is not meant to be reused in a "bigger soft" (like a library would). Supporting multiple Python version is hence unneeded additional work, since it is very likely it will be useless.

You also need at some point to update the dependencies used to latest versions (in pyproject.toml, but also in .pre-commit-config.yaml). Doing this in every PR is a pain and unneeded, but since this is the initial one I suggest you update dependencies to latest versions now, and we will do it once again just before releasing the first version of this scraper.

Regarding last point, you should definitely have a look at openzim/_python-bootstrap#45

[benoit74]

Nota: I've tried to configure CODECOV_TOKEN again, and even added the recommended slug to configuration, but it does not help to have succeeding codecov uploads. I recommend to merge without this step succeeding, I wonder if the problem could be that codecov expects to have a first upload from main branch instead of feature branch.

[codecov]

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

[josephlewis42]

You also need at some point to update the dependencies used to latest versions (in pyproject.toml, but also in .pre-commit-config.yaml). Doing this in every PR is a pain and unneeded, but since this is the initial one I suggest you update dependencies to latest versions now, and we will do it once again just before releasing the first version of this scraper.

Does dependabot work with how these repos are set up or is there a strong preference for using specifically tagged versions?

[benoit74]

Does dependabot work with how these repos are set up or is there a strong preference for using specifically tagged versions?

I don't know, I've never tested, but I think dependabot should work, at least I don't see why I wouldn't.

There is however indeed a strong preference for using specifically pinned versions, because experience showed that we lack sufficient resources to properly test dependency version updates on a daily basis, and scrapers usually lack proper unit / integration testing. So we prefer to update manually when we have time to focus on the update.

We are not against using dependabot, but the prerequisite is obviously that we have sufficient unit / integration testing to ensure version updates are good to merge and not breaking the scraper.

In the past we've got too many issues where someone wanted to contribute on a scraper but first had to waste time fixing problems linked to dependency updates. This make the entry barrier pretty high, since this is quite often subtle bugs, sometimes not even causing a real error but only a non-functional ZIM.

[kelson42]

I would recommend to make a basic README file similar to in other repos.

[benoit74]

LGTM, only one last thing I failed to find in first review, sorry for that.

Since otherwise everything is OK, please directly do the modification and rewrite the commit history of your branch to only keep meaningful commits (i.e. squash / fixup commits). The goal is to remove commits like "Fixed PR comments". These commits are more linked to the review process and won't bring much value / make maintenance harder in one or two years ... This will obviously require a force push on your branch, but this is OK once review is over (or mostly over) - but avoid to do this before the end of the review, since otherwise it makes the second review harder.

[benoit74]

I would recommend to make a basic README file similar to in other repos.

You mean, something more targeted towards usage by everyone rather than how to develop? I considered this would come in another PR once the software has settled, I don't think we already know how the software will really work / which parameters will be necessary. But yes, this is going to be needed.

[josephlewis42]

This will obviously require a force push on your branch, but this is OK once review is over (or mostly over) - but avoid to do this before the end of the review, since otherwise it makes the second review harder.

Done! I didn't realize that "Squash and Merge" was disabled on this repo.

I would recommend to make a basic README file similar to in other repos.

I added a bit more here with some example flags and installation instructions. There's still the warning that they're for example purposes only.

[benoit74]

LGTM, thank you !