knowark / facturark

Facturación Electrónica en Python
GNU Lesser General Public License v3.0
3 stars 4 forks source link

Docs and tests #8

Open blaggacao opened 5 years ago

blaggacao commented 5 years ago

Just some small fixes which should streamling tox testing and general development.

I included pre-commit hooks, please have a look at the respective commit message for detailed information look at the .pre-commit-config.yaml.

I will add one extra commit which will transform styling according to black, if that's too much for your taste just ignore the pre-commit parts. However I found them useful in other projects...

blaggacao commented 5 years ago

@tebanep Don't be scared by the huge changeset of https://github.com/nubark/facturark/pull/8/commits/2520495c960ffaad4a2ddb2057853229b9c97b4d it's fully automatically done by the different pre-commit hooks (notably black) and won't break anything with 99% security.

Here is the output of flake8 and bugbear that seem could need some attention:

Flake8...................................................................Failed
hookid: flake8

facturark/client/client.py:16:72: B006 Do not use mutable data structures for argument defaults. All calls reuse one instance of that data structure, persisting changes between them.
facturark/signer/signer.py:64:95: B950 line too long (94 > 80 characters)
facturark/signer/signer.py:102:9: F841 local variable 'signed_document_element' is assigned to but never used
tests/imager/test_imager.py:9:5: F841 local variable 'message' is assigned to but never used
tests/signer/test_verifier.py:43:9: F841 local variable 'result' is assigned to but never used
tests/test_main.py:100:5: F841 local variable 'result' is assigned to but never used
tests/test_main.py:181:63: F821 undefined name 'mock_build_document'
tests/test_main.py:193:5: F821 undefined name 'cli_build_invoice'
tests/test_utils.py:56:9: F841 local variable 'child' is assigned to but never used
tests/test_utils.py:62:5: F841 local variable 'text' is assigned to but never used
tests/test_utils.py:74:9: F841 local variable 'child' is assigned to but never used
blaggacao commented 5 years ago

Tests are failing (as expected and reproduced locally) on py27, but I think they never did actually work...

Maybe you can use pasterurize from python-future to provide the required compatibility layer, massively and automatically.

blaggacao commented 5 years ago

@tebanep I'm getting excited :+1:

tebanep commented 5 years ago

Tests are failing (as expected and reproduced locally) on py27, but I think they never did actually work...

Maybe you can use pasterurize from python-future to provide the required compatibility layer, massively and automatically.

We'd rather handle compatibility manually, as we are aiming to support python2 just for the next year. It will be history after that, anyway. :)

blaggacao commented 5 years ago

I see.

Out of the series of commits in here just let me know which one are interesting to you and I'll rebase and prepare a proper PR to your liking.

blaggacao commented 5 years ago

If you tell me which of those commits are to your liking, I'll just do a quick cleanup and rebase properly and cleanup the other PRs ...

codecov[bot] commented 5 years ago

Codecov Report

Merging #8 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master     #8   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          94     94           
  Lines        1954   1954           
  Branches       96     96           
=====================================
  Hits         1954   1954

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a0b3068...72aa630. Read the comment docs.

blaggacao commented 5 years ago

@tebanep I reduced the diff to the minimum sensible proposal.

This way, you can run pre-commit run --all at any time you want (and where it makes sense to a clean history). Accepting this PR, going forward hooks are only executed on file diffs (and only if locally initialized)

tebanep commented 5 years ago

@tebanep I reduced the diff to the minimum sensible proposal.

* No p27 fixing

* only include pre-commit hooks

* add flake8 config file

This way, you can run pre-commit run --all at any time you want (and where it makes sense to a clean history). Accepting this PR, going forward hooks are only executed on file diffs (and only if locally initialized)

@blaggacao Thanks a lot for your commitment towards this project! Give me until the weekend to review your proposal in calm as I think I need a little bit more of background. :+1: