app-office/visidata: bump to 2.1.1

[ezzieyguywuf]

Signed-off-by: Wolfgang E. Sanyer WolfgangESanyer@gmail.com

Added some new optional dependencies as well. Test suite runs and completes
succesfully - I had to disable a few tests.

[AndrewAmmerlaan]

This looks good overall, I left some comments mostly about the tests part.

[AndrewAmmerlaan]

According to https://github.com/saulpw/visidata/blob/stable/dev/requirements-dev.txt this should also depend on pytest. The python_enable_tests pytest function will automatically add this dependency, and will also automatically add IUSE="test" and RESTRICT="!test? ( test )" so you can than remove those lines as well.

[epsilon-0]

RDEPENDS is also not needed, it is automatically assumed when doing tests.

[ezzieyguywuf]

this should also depend on pytest

Ah, I missed this, as I only diffed the requirements.txt from the top-level directory with the version from the previous ebuild. If I had diffed this sub-requirements.txt I would have seen the new dep. I'll make sure to add it.

RDEPENDS is also not needed,

Hm, this is also an artifact of the original ebuild. I have no problem taking it out.

[ezzieyguywuf]

RDEPENDS is also not needed, it is automatically assumed when doing tests.

hm, actually, is this true? I see that RDEPEND is added automatically when distutils_enable_tests pytest is used, but that's not being used here.

[epsilon-0]

you should use it and then remove RDEPEND.

[ezzieyguywuf]

but how can I use distutils_enable_tests if I need to do all this weird git init stuff and then manually execute dev/tests.sh to run tests?

[epsilon-0]

distutils_enable_tests provides a default impl of python_test (in addition to doing the other things of IUSE and DEPEND/RDEPEND). As you are overriding the function, distutils_enable_tests wont change the python_test for you.
But you can still take advantage of the other things done by distutils_enable_tests, which are the dependency additions and USE flag fixing.

[ezzieyguywuf]

So this is the preferred approach? It seems a bit roundabout, especially since distutils_enable_tests requires a target - what should I do, pick a random one? distutils_enable_tests pytest? But this is misleading to future maintainers - I'm not using pytest, I'm using a custom test script.

[AndrewAmmerlaan]

I'm not using pytest, I'm using a custom test script.

I'm pretty sure the custom test script calls pytest at some point, since pytest is specified as a dependency by upstream after all.

[AndrewAmmerlaan]

I'm not using pytest, I'm using a custom test script.

I'm pretty sure the custom test script calls pytest at some point, since pytest is specified as a dependency by upstream after all.

Indeed it does:

andrew@andrew-gentoo-pc visidata-stable % grep -r pytest
visidata/tests/test_path.py:import pytest
visidata/tests/test_edittext.py:import pytest
visidata/tests/test_edittext.py:    @pytest.fixture(autouse=True, scope='function')
visidata/tests/test_edittext.py:    @pytest.mark.parametrize('keys, result, kwargs', [
visidata/tests/test_edittext.py:            with pytest.raises(exception):
visidata/tests/test_commands.py:import pytest
visidata/tests/test_commands.py:@pytest.mark.usefixtures('curses_setup')
visidata/tests/conftest.py:import pytest
visidata/tests/conftest.py:@pytest.fixture(scope="class")
visidata/tests/conftest.py:@pytest.fixture(scope="function")

[ezzieyguywuf]

This looks good overall, I left some comments mostly about the tests part.

Thanks for the review! I'll take some time later to work through your comments.

[AndrewAmmerlaan]

Thanks, Merged :)

I added DISTUTILS_USE_SETUPTOOLS=rdepend to get rid of the warning shown in the pkg_postinst phase.