Fix version compatibility test.

[sudden6]

When the version number has trailing zeros, a change of the first non-zero number means a break in API compatibility. eg. 0.0.1 to 0.0.2 means a break, 0.3.0 to 0.4.0 means a break, but 0.3.0 to 0.3.1 means not.

---

This change is 

[iphydf]

@sudden6 I've added some tests. I think these tests should pass, but they don't. They don't pass on the old macro either, so it seems the old macro was broken to begin with. Do you think these tests should pass? Also, if you can, please add some tests of your own, as these are currently incomplete.

[iphydf]

Unassigning myself, since I rewrote the version check macro. Someone else needs to review.

[robinlinden]

---

Reviewed 3 of 5 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks broke.

---

Comments from Reviewable

[GrayHatter]

Reviewed 1 of 5 files at r1.
Review status: 4 of 5 files reviewed at latest revision, 3 unresolved discussions.

---

CMakeLists.txt, line 430 at r3 (raw file):

  auto_test(toxav_many)
endif()
auto_test(version)

move this above if(BUILD_TOXAV) and add a blank line between the must run, and optionally run

---

toxcore/tox.api.h, line 193 at r3 (raw file):

  (TOX_VERSION_MAJOR > 0 && TOX_VERSION_MAJOR == MAJOR) && (		\
    /* 1.x.x, 2.x.x, etc. with matching major version. */		\
    TOX_VERSION_MINOR > MINOR ||					\

tabs and not spaces?

---

toxcore/tox.h, line 195 at r3 (raw file):

#define TOX_VERSION_IS_API_COMPATIBLE(MAJOR, MINOR, PATCH)          \
  (TOX_VERSION_MAJOR > 0 && TOX_VERSION_MAJOR == MAJOR) && (        \
    /* 1.x.x, 2.x.x, etc. with matching major version. */       \

Comments don't belong inside a macro, around is fine.

If that's a problem, the macro is too big.

---

Comments from Reviewable

[GrayHatter]

Reviewed 2 of 3 files at r4.
Review status: 4 of 5 files reviewed at latest revision, 4 unresolved discussions.

---

auto_tests/version_test.c, line 18 at r3 (raw file):

{
    if (actual != expected) {
        printf("Client version %d.%d.%d is%s compatible with library version %d.%d.%d, but it should%s be\n",

this hurts readability, this should be another if

tests should be 'glance-able'

---

Comments from Reviewable

[iphydf]

Review status: 4 of 5 files reviewed at latest revision, 4 unresolved discussions.

---

CMakeLists.txt, line 430 at r3 (raw file):

Previously, GrayHatter (Gregory Mullen) wrote…

move this above if(BUILD_TOXAV) and add a blank line between the must run, and optionally run

Done.

---

toxcore/tox.api.h, line 193 at r3 (raw file):

Previously, GrayHatter (Gregory Mullen) wrote…

tabs and not spaces?

Done.

---

toxcore/tox.h, line 195 at r3 (raw file):

Previously, GrayHatter (Gregory Mullen) wrote…

Comments don't belong inside a macro, around is fine.

If that's a problem, the macro is too big.

The macro is necessarily big. The logic is complicated. We can't make it smaller, because this needs to work in the preprocessor. The only way to make this smaller is by making helper macros, and those won't help maintainability.

---

Comments from Reviewable

[GrayHatter]

---

Reviewed 1 of 5 files at r1, 1 of 3 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

toxcore/tox.h, line 195 at r3 (raw file):

Previously, iphydf wrote…

The macro is necessarily big. The logic is complicated. We can't make it smaller, because this needs to work in the preprocessor. The only way to make this smaller is by making helper macros, and those won't help maintainability.

That's what I was suggesting, and it's what I would have done myself.

Either way, for what it is, with a test to back it up LFTM (looks fine to me)

---

Comments from Reviewable

[robinlinden]

This doesn't have any tests covering the case where a client requests a major version < TOX_VERSION_MAJOR, so that might be something we'll want to add.

[nurupo]

Reviewed 1 of 5 files at r1, 3 of 3 files at r4, 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

toxcore/tox.h, line 197 at r5 (raw file):

    /* 1.x.x, 2.x.x, etc. with matching major version. */               \
    TOX_VERSION_MINOR > MINOR ||                                        \
    TOX_VERSION_MINOR == MINOR && TOX_VERSION_PATCH >= PATCH            \

I'd add () around TOX_VERSION_MINOR == MINOR && TOX_VERSION_PATCH >= PATCH just to make it more clear (not everyone remembers order precedence of operators).

---

Comments from Reviewable