rename validate_zimfile_creatable to validate_file_creatable

openzim / python-scraperlib

Collection of Python code to re-use across Python-based scrapers

GNU General Public License v3.0

20 stars 18 forks source link

rename validate_zimfile_creatable to validate_file_creatable #210

Closed elfkuzco closed 2 weeks ago

elfkuzco commented 2 weeks ago

Rationale

This PR renames the validate_zimfile_creatable method to validate_file_creatable to reflect its general applicability to check file creation beyond ZIM files. This resolves #200

Changes

Add validate_folder_creatable function to check if a folder can be written to.
Add validate_file_creatable function to check if a file with a given filename can be created in a given folder
Delegate validate_zimfile_creatable to use validate_file_creatable method with deprecation warning.
Add tests for new functions

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (ea6505f) to head (baa3e9d). Report is 4 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #210 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 38 38 Lines 2221 2223 +2 Branches 426 426 ========================================= + Hits 2221 2223 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

rgaudin commented 2 weeks ago

Yes, fine with me.

I'd suggest we use a dedicated subsection in the changelog (### Breaking Changes instead of prefixing individual changes with BREAKING). I find is more practical

benoit74 commented 2 weeks ago

I'd suggest we use a dedicated subsection in the changelog (### Breaking Changes instead of prefixing individual changes with BREAKING). I find is more practical

Agreed. Let's add it to the bootstrap: https://github.com/openzim/_python-bootstrap/pull/50

elfkuzco commented 2 weeks ago

I propose that we consider this as a real breaking change and:

* rename all exceptions to remove any "ZIM" reference in their name (and clearly document these renaming in the changelog so that it is easier for scraperlib users to know about it)

* completely drop the `validate_zimfile_creatable` method

* bump version in `__about__.py` to `5.0.0-dev0` to indicate this will need a major (we already had doubts about whether other changes of the scraperlib deserved a major or not, this is somehow providing a good answer ^^)

@benoit74 , should I implement these on top of this PR?