0010 - Module Versions bump convention following PS core min version

[matks]

No description provided.

[matks]

I think it’s confusing that first number doesn’t mean what it supposed to mean, I’m from the school where if you bump major version there are some breaking changes, if you bump minor version there are some new features and improvements, and of course path versions means some bugfixes etc.

Isn't it exactly the definition of SemVer 😅 ?

Given a version number MAJOR.MINOR.PATCH, increment the:

MAJOR version when you make incompatible API changes,
MINOR version when you add functionality in a backwards compatible manner, and
PATCH version when you make backwards compatible bug fixes.

[jolelievre]

I also think we consider this dependency in a wrong way, we are not talking about a dependency that is used by the module, which is bundled in the module and which the module updates itself

PrestaShop is not just any dependency for a module, it's the WHOLE system in which the module can work. As such I'm more referring to the initial definition in semver:

MAJOR version when you make incompatible API changes,
MINOR version when you add functionality in a backwards compatible manner, and

When we make a change in the module that rely on a feature that is only available in a new version of PS (let's say 1.7.7), then we change the module API which can not work on previous versions, it is a BC break. Just like when we change something in the core API we create BC change.

So in my opinion creating a BC implies bump to the next major version, it's as simple as that. It's not a semantic question about dependency or what not. It does not work any more!!

So if we need a more recent version of PrestaShop => bump to major

[jolelievre]

However if we handled the modification depending on the core version (by comparing versions), which much more complicated to do but some core modules do it, and many external developers handle such backward compatibilities in their module with compatibility ranges sometimes from 1.5 to 1.7.7!!! In this case since the backward compatibility is respected it's bump to minor.

[jolelievre]

I don't think we ever have such precise dependencies, we usually only set dependencies based on minor at best.

[matks]

I had to mention this usecase, I know some of our colleagues are all about details 😏

[jolelievre]

I would like such behaviour, but I'd say it is a bit biased by our way of handling semver.. Based on semver we only perform "patch" versions 😅 (third digits) But let's not stop on this and consider that 1.7.6 to 1.7.7 is indeed a minor update (as we consider it).

We only perform "minor updates" since the beginning of 1.7 but it's barely a minor update.. We limit the BC changes as much as possible but we do introduce some. We also integrate new tools, APIs and features which may be mandatory for modules when they start using it (Doctrine entities, _legacy_link, modern controllers, symfony services, ...)

These are big (major?) changes in the architecture, and module can't afford to be backward compatible without some of these features, nor can they integrate libraries as fallback to handle them. So I'd still say it's a major change in the module API which implies a major bump.

[kpodemski]

I would like such behaviour, but I'd say it is a bit biased by our way of handling semver.. Based on semver we only perform "patch" versions 😅 (third digits)

But... why? 🤔 Maybe I'm missing something, but why do we need to bump module (plugin) version only because there's a path version of the core software?

[matks]

@jolelievre To clarify: when I mention PS Core minor or major updates, I'm talking about the numbers after the 1.

So 1.7.3.4 is 7.3.4 for me. I ignore the 1 which has no meaning.

So if I mention a minor bump, it's like 1.7.5 to 1.7.6 for me

[matks]

@jolelievre @kpodemski @PierreRambaud ADR updated to be more clear, and added a candidate suggested by @PierreRambaud a82854d

[kpodemski]

@matks

@PierreRambaud suggests to stop using SemVer for modules and modify upon release the version number according to the type of changes introduced: bug fixes (patch), or new features (minor), or important changes (major).

So if I'm this understanding this correctly, if we release v1.7.8, and... we have somemodule v1.0.0... and...

• no changes in somemodule, it still stays at 1.0.0
• bugfixes in somemodule bump to 1.0.1
• improvements in somemodule bump to 1.1.0
• BC in somemodule bump to v2

yes?

[matks]

@kpodemski

If we exclude the "new module version bumps minimum PS version" usecases, @PierreRambaud is even simpler:

• no changes in somemodule, it still stays at 1.0.0
• bugfixes in somemodule bump to 1.0.1
• improvements in somemodule bump to 1.1.0
• big changes in somemodule bump to v2

@PierreRambaud suggests to completely forget about BC breaks for modules because they are not useful for modules. Just use "patch", "minor" and "major" to refer to the size/impact of the changes. So improvements with BC breaks could be done in a minor version.

[matks]

@PrestaShop/prestashop-maintainers Can we host a vote or do we wait for January?

[PierreRambaud]

@PrestaShop/prestashop-maintainers Can we host a vote or do we wait for January?

Since everyone is on vacation, we can wait for January 😅

[eternoendless]

IMO the minor/major question is mostly about compatibility with PrestaShop so that we can update modules in patch versions.

PrestaShop version 1.7.7.0 imports somemodule@^2.0. This allows any minor and patch versions of this module to be imported in subsequent patch releases of 1.7.7.x. If the module needs to increase its minimum compatible PrestaShop version, then it will require a major version release to avoid it being imported by composer into a PrestaShop version that won't support it.

[matks]

IMO the minor/major question is mostly about compatibility with PrestaShop so that we can update modules in patch versions.

PrestaShop version 1.7.7.0 imports somemodule@^2.0. This allows any minor and patch versions of this module to be imported in subsequent patch releases of 1.7.7.x. If the module needs to increase its minimum compatible PrestaShop version, then it will require a major version release to avoid it being imported by composer into a PrestaShop version that won't support it.

So if I get it you are in favor of my initial architecture change suggestion? 😉 (before the change suggested by @PierreRambaud )

[eternoendless]

I'm in favor of this (and only this) decision:

WHEN a new module version requires to bump the PrestaShop minimum version,

IF the bump is minor or major (ex: PS 1.7.6.2 => 1.7.7.0)
THEN the module version must be bumped to at least the next major version (ex: ps_searchbar 2.0.1 => 3.0.0)

This part has never happened before AFAIK:

IF the core bump is a patch bump (ex: PS 1.7.6.2 => 1.7.6.4)
THEN the module version must be bumped to at least the next minor version (ex: ps_searchbar 2.0.1 => 2.2.0)

Let's keep the consensual part only to move the decision forward.

BTW I have the feeling that this ADR is too argumentative. I think ADRs should be simple and objective for readability's sake: describe the problem (context) -> record decision, that's all. Keep it objective, leave the use of "I", opinions and discussion for review / issue.

[tswfi]

as a side note, modules also have a ps_versions_compliancy for telling which versions it works, but even core modules do not respect it but default for the max_version being the currently installed version: https://github.com/PrestaShop/ps_checkpayment/blob/dev/ps_checkpayment.php#L67

[matks]

ADR process is onerous to follow and I will not have the time to bring this ADR to approval point. I close it as the topic was not a critical aspect.

If anybody is interested in the topic, feel free to use my work to submit a contribution.