search

openzim / _python-bootstrap

Sample openZIM Python project bootstrap
1 stars 2 forks source link

Minor tasks changes and Docker sample for daemon #26

Closed benoit74 closed 1 year ago

benoit74 commented 1 year ago

Fix #25

Changes

codecov[bot] commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (0010546) 100.00% compared to head (5d7a537) 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #26 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 1 1 Lines 8 8 Branches 1 1 ========================================= Hits 8 8 ```

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

benoit74 commented 1 year ago

Sorry for the force-push a bit late, but I realized that I made a mistake

rgaudin commented 1 year ago

PR name is not very useful neither 😉

benoit74 commented 1 year ago

PR name is not very useful neither 😉

Agreed, but what would you suggest? Changes are really not related at all ... so I struggle to find something short and useful

benoit74 commented 1 year ago

I removed most changes, see the updated list of changes before reviewing.

I do not achieve to reproduce your issue regarding pytest escaping a non activated virtualenv, so I isolated this change in a specific issue: https://github.com/openzim/_python-bootstrap/issues/27

benoit74 commented 1 year ago

I finally understood why we needed sometimes the args = args or "." thing.

It is needed like you said for hatch scripts which passes --args '{args}', i.e. --args '' if no args are provided. We hence always get a string (no need for str | None, str is OK) but need the args = args or "." for tasks which expects a path (and not for the ones which are ok with an empty arg).

Please review again to confirm it is ok for you before merging, sorry for that late understanding, but at least it is now correctly typed + documented.