search

chrislim2888 / IP2Location-C-Library

IP2Location C library enables the user to find the country, region, city, coordinates, zip code, time zone, ISP, domain name, connection type, area code, weather station code, weather station name, mobile, usage types, etc that any IP address or hostname originates from.
https://www.ip2location.com
MIT License
61 stars 30 forks source link

8.3.1 library reports as 8.3.0 #41

Closed remicollet closed 3 years ago

remicollet commented 3 years ago

https://github.com/chrislim2888/IP2Location-C-Library/blob/master/libIP2Location/IP2Location.h#L57

pbiering commented 3 years ago

API hasn't changed, therefore 8.3.0 should be still ok imho IP2Location_lib_version_string should return 8.3.1

ip2location commented 3 years ago

In this case, I have bumped up the version to 8.3.1.

pbiering commented 3 years ago

I don't understand that change!

There is not any line changed in API code of the library...why is then the API version increased?

My understanding from a developer's point of view using that library "optional" by dynamic link loading and potentially support several minor or even major versions by checking the API version number:

Optimizations, cosmetic fixes, code changes not affecting any existing API library-version++ api-version: equal

Adding new APIs without breaking exisiting APIs: library-version++ api-version++

Breaking existing APIs: library-version++ api-version++ soname++

has someone a different understanding?

remicollet commented 3 years ago

Probably this macro is badly named, as it is the library version, I don't see how it is related to API. So IMHO, it have to match the library version.

The API version is more described by the soname (which should have be bump to 3.1 as new symbols were added in 8.3.0, so -version-info 4:0:1, but not critical, and too late)

pbiering commented 3 years ago

afair I've requested introducting lib-version in the past decoupled from API version to get indication about in case the API was stable while the library version was updated (similar to GeoIPv1 library afair. This were the times before soname was introduced...

So you recommend?

Optimizations, cosmetic fixes, code changes not affecting any existing API library-version++ / api-version: ++

Adding new APIs (or symbols) without breaking exisiting APIs: library-version++ / api-version++ soname.minor++

Breaking existing APIs (or symbols): library-version++ / api-version++ soname.major++

remicollet commented 3 years ago

Optimizations, cosmetic fixes, code changes not affecting any existing API

API_VERSION_RELEASE++

Adding new APIs (or symbols) without breaking exisiting APIs:

API_VERSION_MINOR++ soname.minor++

Breaking existing APIs (or symbols):

API_VERSION_MAJOR++ soname.major++

With API_VERSION = library version

remicollet commented 3 years ago

See https://semver.org/

remicollet commented 3 years ago

Notice: it may be needed to check library patch number in code, even for minor changes not affecting API

Example: https://github.com/php/php-src/blob/PHP-7.4/ext/zip/php_zip.c#L1038 (ok, libzip 1.3.1 have introduced a bad free issue... which was quickly fixed in 1.3.2)