search

iwd-nl / snelstart-php

PHP 7.2+ Library for the Snelstart API https://b2bapi-developer.snelstart.nl/
7 stars 22 forks source link

Remove legacy V1 code #36

Open manuelderuiter opened 1 year ago

manuelderuiter commented 1 year ago

Since V1 has been deprecated for a while now I think it's time to bump the version of the library and remove old (legacy) code. This will ensure we can use the latest deps and perhaps even factor in some really overdue changes like listed below:

Some of the tasks:

HVSoftware commented 1 year ago

I didn't saw this issue. i saw your request and I started with the removal of the V1 classes. See https://github.com/iwd-nl/snelstart-php/pull/37.

Maybe it's also wise to select a code standard when we are using php cs fixer. What will be the minumum cs we need to support?

HVSoftware commented 1 year ago

@manuelderuiter I see you mentioned the removal of class aliases. What do you mean with remove class aliases? I see there is a V2 namespace used in the code. See below.

namespace SnelstartPHP\Request\V2;

Do you want the V2 namespace to be removed, like shown below? This will move all classes to one folder below the V2 folder. The V1 namespace is remove by another PR.

namespace SnelstartPHP\Request;

As a result the following items should be updated:

Can you confirm this is needed. I'm able to make these updates.

rojtjo commented 1 year ago

Might be better to just leave it in the V2 namespace in my opinion. It's backward compatible and if SnelStart ever decides to introduce V3 it can be easily added next to V2.

manuelderuiter commented 1 year ago

I do agree with the backward compatible part but I don't think we should support multiple versions of the same API for Snelstart or any other lib. The amount of test cases have to be doubled and the overall quality of the lib will go down. I think it will be easier to have multiple branches with different code than to have one branch that handles multiple API versions.

In case of a new API version we can easily bump the tag to a new major version to avoid confusion.

HVSoftware commented 1 year ago

There is no need to change the code if there is no need to remove the V2 namespace. Also look at the work-not-to-be-done. I think we can mark the second tasks as handled.

HVSoftware commented 1 year ago

We can prepare the code using Rector. See my PR.

PHP 7.2. is supported. Now we can let Rector take care of updating to 8.0, 8.1 and 8.2. Do we need support of PHP 8.0?

HVSoftware commented 1 year ago

I've update the code to support PHP 7.3. This has been done to see what is needed.

https://github.com/iwd-nl/snelstart-php/pull/42

The same has been done to support PHP 7.4.

https://github.com/iwd-nl/snelstart-php/pull/43

This will show some of the advantage of Rector.

HVSoftware commented 1 year ago

The following PR can solve the following tasks (https://github.com/iwd-nl/snelstart-php/pull/37):