zivid / zivid-python

Official Python package for Zivid 3D cameras
BSD 3-Clause "New" or "Revised" License
40 stars 14 forks source link

Simplify test matrix table #191

Closed eskaur closed 1 year ago

eskaur commented 1 year ago

Listing the SDK version in the test matrix is not really useful. Each commit of zivid-python supports exactly one version of the SDK, and that version is stated further up in the README anyway. Simplifying this will reduce the manual work of bumping the supported SDK version.

eskaur commented 1 year ago

I agree, the Wrapper has different installable versions that support different versions of the SDK. However, If I merely check out master branch and do pip install ., that will only work if the installed SDK version exactly matches what is stated on the top of the README. To support other SDKs you have to fiddle with setup.py.

This table is as I understand it, not about what is possible, but about what we actually test.

Exactly. And we always test against the SDK version that is stated at the top of the README. In practice, whenever I bump the SDK version on master, I have to search&replace 10ish copies of the same number, and they are always the same (correct me if I'm wrong). My intent here was merely to reduce duplication.

What if we between the Test matrix and the table put This version of zivid-python is tested against Zivid SDK 2.8.0? Then the number is only stated twice, not ten times.

nedrebo commented 1 year ago

However, If I merely check out master branch and do pip install .,

Installing from repo is not recommend in the readme, it recommends pip install zivid

To support other SDKs you have to fiddle with setup.py.

The readme say that pip install zivid=1.2.3.4.5.6 works to select SDK=4.5.6, is that not true?

What if we between the Test matrix and the table put This version of zivid-python is tested against Zivid SDK 2.8.0?

I'm fine with that. Given, that is is true. You did not answer if this is dynamic or hard-coded. Do you know?

For Arch Linux at least, we do test against the latest released SDK. If we, for some reason, do not update the wrapper immediately after a release, the readme will be (and is in the current version) wrong.

nedrebo commented 1 year ago

If I merely check out master branch and do pip install .,

I don't think this is "merely". I think most users get this from PyPi and never visit GitHub.

PyPi also displays the Readme, so it must also (and even more importantly) make sense there.

eskaur commented 1 year ago

I guess it depends on this question: Does the Readme.md on a commit represent just that commit, or all previous releases?

Installing from repo is not recommend in the readme, it recommends pip install zivid

The readme actually mentions both ways. There's one header for each.

The readme say that pip install zivid=1.2.3.4.5.6 works to select SDK=4.5.6, is that not true?

That is true.

I'm fine with that. Given, that is is true. You did not answer if this is dynamic or hard-coded. Do you know?

The SDK version tested in the CI is hard-coded in the repo with explicit links to installers.

For Arch Linux at least, we do test against the latest released SDK.

This is not actually true. There is a hardcoded reference to a specific version.