search

HalfDeadPie / labelord_halfdeadpie

v0.5
Creative Commons Zero v1.0 Universal
0 stars 0 forks source link

Tests feedback #5

Open MarekSuchanek opened 6 years ago

MarekSuchanek commented 6 years ago

Solution is nice but there are few minor things:

  1. Build artifacts should now be in repo (.egg, build, __pycache__, etc.)
  2. It would be better to use additional cassette matchers, sometimes matching just URI+Method might not be enough...
  3. Tests + cassettes should be in distributed package (if you do python setup.py sdist so in the content of produced .tar.gz, but should not install) - it is violating task assigment, if not fixed I will have to -0.5 points.
  4. There is no "tests_unit/conftest_unit.py", probably mistyped
MarekSuchanek commented 6 years ago

Thanks for fixing few of mentioned but build artifacts are still there... now even .cache... So I give 4.8 points.

HalfDeadPie commented 6 years ago

Ok. I would like to ask you. Should I remove the artifacts or not next time? Because you have suggested adding them to repo in previous message.

Solution is nice but there are few minor things:

1. Build artifacts should now be in repo (.egg, build, __pycache__, etc.)
2. It would be better to use additional cassette matchers, sometimes matching just URI+Method might not be enough...
2. Tests + cassettes should be in distributed package (if you do python setup.py sdist so in the content of produced .tar.gz, but should not install) - it is violating task assigment, if not fixed I will have to -0.5 points.
3. There is no "tests_unit/conftest_unit.py", probably mistyped

So that is the reason why I have added .cache also. Have I misunderstood?

MarekSuchanek commented 6 years ago

ouch... sorry for that... "shouldn't be in repo" as we also often mention during tutorials... so please remove it and I fix score to 5 points

HalfDeadPie commented 6 years ago

They should be deleted now.