workflow_script icon indicating copy to clipboard operation
workflow_script copied to clipboard

Does the App follow semantic versioning?

Open martin-rueegg opened this issue 3 years ago • 3 comments

Workflow Script app

Workflow Script app version: 1.9.0

https://github.com/nextcloud/workflow_script/commit/44f73abcda15d2cc0b6e757695002efae61c0afa seems to introduce several breaking changes.

E.g. in Launcher.php the signature of OCA\WorkflowScript\BackgroundJobs\Launcher's constructor changes from

public function __construct(LoggerInterface $logger, ITempManager $tempManager, IRootFolder $rootFolder)

to

public function __construct(ITimeFactory $time, LoggerInterface $logger)

Similarly affected is OCA\WorkflowScript\Operation and more subtly OCA\WorkflowScript\Listener\RegisterFlowOperationsListener where only the interface of the parameter changes.

Do these changes not violate the backwards-compatibility requirement, that is implied by the same major version?

Hence, should version 1.9.0 not have been 2.0.0 instead?

The whole idea of semantic versioning is to avoid code breaks on dependencies. According to the (developer documentation)[https://nextcloudappstore.readthedocs.io/en/latest/developer.html#info-xml], version numbers must follow semantic versioning.

martin-rueegg avatar Aug 03 '22 11:08 martin-rueegg

Hi @martin-rueegg

There may be room for improvement, but - unless I'm mistaken - the things you mentioned above aren't what I'd call public facing APIs.

Did you encounter a problem or some unexpected behavior in your use case that prompted this Issue?

There's obviously some judgement involved so maybe something got overlooked.

The docs you linked to are mostly for external developers. The following is for NC developed apps and the versioning management is more explicitly defined there:

https://docs.nextcloud.com/server/latest/developer_manual/app_publishing_maintenance/release_process.html#versioning

joshtrichards avatar Jun 29 '23 20:06 joshtrichards

Hi @joshtrichards

Thank you for getting back at me. Well, I was looking into implementing a picture and video resizing app, that is currently implemented as an external script solution, into an app for nextcloud and hence bilding on top of the Workflow Script.

However, in extending existing classes, it is useful to know, how stable an API is and how breaking changes are announced. Hence the question.

It may be, that for NC environment the semver scheme mainly focuses on the environment an app is running in (this is implied by the documentation of your above mentioned documentation). However, technically, every non-private method is part of an API. And I personally consider a change to a public constructor to be a (breaking) change of the API and hence reflect that in the version number. Even in my internal libraries. Just to make sure a change doesn't get missed. And also for an extension to easier detect which version of a method is to be called.

As such, I would encourage this for any app that implies semver. As otherwise the whole point is lost - if breaking changes are only defined by the compatibility with the environment and not also with extensions.

That are just my 2 cents. :-)

martin-rueegg avatar Jul 03 '23 06:07 martin-rueegg

Hey @martin-rueegg,

this app does not expose any API and any classes that are implemented here are considered to internal.

We also do (try to) avoid inter-app dependencies. For use cases that require apps communicating with each other, APIs in the server shall be defined, or Event classes in either the server or the app.

blizzz avatar Nov 15 '23 13:11 blizzz