inveniosoftware / invenio-records-rest

Invenio records REST API module.
https://invenio-records-rest.readthedocs.io
MIT License
4 stars 62 forks source link

global: release-checklist #161

Closed hjhsalo closed 6 years ago

hjhsalo commented 6 years ago

Signed-off-by: Harri Hirvonsalo harri.hirvonsalo@cern.ch

hjhsalo commented 6 years ago

Can the reviewer (or someone else) comment on tests/conftest.py. I think it is not according to https://github.com/inveniosoftware/invenio/wiki/Release-preparation-checklist :

Check instance_path and app fixtures. Should be using @pytest.yield_fixture and app fixture should create application context.

Also is the list in .editorconfig:known_third_party contains more modules than what is included in requirements-devel.txt. Is this ok as https://github.com/inveniosoftware/invenio/wiki/Release-preparation-checklist says the following:

List of modules in.editorconfig:known_third_party should match 1-to-1 all devel requirements.

lnielsen commented 6 years ago

Re. .editorconfig: It's ok to have more modules in .editorconfig than in requirements-devel.txt. The other way around, i.e. module in requirements-devel.txt but not in .editorconfig will cause problems with isort and import sorting order.

Re. test/conftest.py then it could be improved, but it's too much work to change it in this module I think.